Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(v8/sveltekit): Ensure source maps deletion is called after source maps upload #14963

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 85 additions & 4 deletions packages/sveltekit/src/vite/sourceMaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
plugin => plugin.name === 'sentry-vite-debug-id-upload-plugin',
);

const sentryViteFileDeletionPlugin = sentryPlugins.find(plugin => plugin.name === 'sentry-file-deletion-plugin');

const sentryViteReleaseManagementPlugin = sentryPlugins.find(
// sentry-debug-id-upload-plugin was the old (misleading) name of the plugin
// sentry-release-management-plugin is the new name
plugin => plugin.name === 'sentry-debug-id-upload-plugin' || plugin.name === 'sentry-release-management-plugin',
);

if (!sentryViteDebugIdUploadPlugin) {
debug &&
// eslint-disable-next-line no-console
Expand All @@ -85,7 +93,33 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
return sentryPlugins;
}

const restOfSentryVitePlugins = sentryPlugins.filter(plugin => plugin.name !== 'sentry-vite-debug-id-upload-plugin');
if (!sentryViteFileDeletionPlugin) {
debug &&
// eslint-disable-next-line no-console
console.warn(
'sentry-file-deletion-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins',
);
return sentryPlugins;
}

if (!sentryViteReleaseManagementPlugin) {
debug &&
// eslint-disable-next-line no-console
console.warn(
'sentry-release-management-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins',
);
return sentryPlugins;
}

const unchangedSentryVitePlugins = sentryPlugins.filter(
plugin =>
![
'sentry-vite-debug-id-upload-plugin',
'sentry-file-deletion-plugin',
'sentry-release-management-plugin', // new name of release management plugin
'sentry-debug-id-upload-plugin', // old name of release management plugin
].includes(plugin.name),
);

let isSSRBuild = true;

Expand All @@ -95,8 +129,8 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
__sentry_sveltekit_output_dir: outputDir,
};

const customPlugin: Plugin = {
name: 'sentry-upload-sveltekit-source-maps',
const customDebugIdUploadPlugin: Plugin = {
name: 'sentry-sveltekit-debug-id-upload-plugin',
apply: 'build', // only apply this plugin at build time
enforce: 'post', // this needs to be set to post, otherwise we don't pick up the output from the SvelteKit adapter

Expand Down Expand Up @@ -248,7 +282,54 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
},
};

return [...restOfSentryVitePlugins, customPlugin];
// The file deletion plugin is originally called in `writeBundle`.
// We need to call it in `closeBundle` though, because we also postpone
// the upload step to `closeBundle`
const customFileDeletionPlugin: Plugin = {
name: 'sentry-sveltekit-file-deletion-plugin',
apply: 'build', // only apply this plugin at build time
enforce: 'post',
closeBundle: async () => {
if (!isSSRBuild) {
return;
}

const writeBundleFn = sentryViteFileDeletionPlugin?.writeBundle;
if (typeof writeBundleFn === 'function') {
// This is fine though, because the original method doesn't consume any arguments in its `writeBundle` callback.
const outDir = path.resolve(process.cwd(), outputDir);
try {
// @ts-expect-error - the writeBundle hook expects two args we can't pass in here (they're only available in `writeBundle`)
await writeBundleFn({ dir: outDir });
} catch (e) {
// eslint-disable-next-line no-console
console.warn('Failed to delete source maps:', e);
}
}
},
};

const customReleaseManagementPlugin: Plugin = {
name: 'sentry-sveltekit-release-management-plugin',
apply: 'build', // only apply this plugin at build time
enforce: 'post',
closeBundle: async () => {
try {
// @ts-expect-error - this hook exists on the plugin!
await sentryViteReleaseManagementPlugin.writeBundle();
} catch (e) {
// eslint-disable-next-line no-console
console.warn('[Source Maps Plugin] Failed to upload release data:', e);
}
},
};

return [
...unchangedSentryVitePlugins,
customReleaseManagementPlugin,
customDebugIdUploadPlugin,
customFileDeletionPlugin,
];
}

function getFiles(dir: string): string[] {
Expand Down
10 changes: 6 additions & 4 deletions packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,13 @@ describe('sentrySvelteKit()', () => {
// default source maps plugins:
'sentry-telemetry-plugin',
'sentry-vite-release-injection-plugin',
'sentry-debug-id-upload-plugin',
'sentry-vite-debug-id-injection-plugin',
'sentry-file-deletion-plugin',
// custom release plugin:
'sentry-sveltekit-release-management-plugin',
// custom source maps plugin:
'sentry-upload-sveltekit-source-maps',
'sentry-sveltekit-debug-id-upload-plugin',
// custom deletion plugin
'sentry-sveltekit-file-deletion-plugin',
]);
});

Expand All @@ -76,7 +78,7 @@ describe('sentrySvelteKit()', () => {
const instrumentPlugin = plugins[0];

expect(plugins).toHaveLength(1);
expect(instrumentPlugin.name).toEqual('sentry-auto-instrumentation');
expect(instrumentPlugin?.name).toEqual('sentry-auto-instrumentation');

process.env.NODE_ENV = previousEnv;
});
Expand Down
146 changes: 129 additions & 17 deletions packages/sveltekit/test/vite/sourceMaps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,31 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { Plugin } from 'vite';
import { makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps';

const mockedSentryVitePlugin = {
const mockedViteDebugIdUploadPlugin = {
name: 'sentry-vite-debug-id-upload-plugin',
writeBundle: vi.fn(),
};

const mockedViteReleaseManagementPlugin = {
name: 'sentry-release-management-plugin',
writeBundle: vi.fn(),
};

const mockedFileDeletionPlugin = {
name: 'sentry-file-deletion-plugin',
writeBundle: vi.fn(),
};

vi.mock('@sentry/vite-plugin', async () => {
const original = (await vi.importActual('@sentry/vite-plugin')) as any;

return {
...original,
sentryVitePlugin: () => [mockedSentryVitePlugin],
sentryVitePlugin: () => [
mockedViteReleaseManagementPlugin,
mockedViteDebugIdUploadPlugin,
mockedFileDeletionPlugin,
],
};
});

Expand All @@ -30,20 +44,22 @@ beforeEach(() => {
vi.clearAllMocks();
});

async function getCustomSentryViteUploadSourcemapsPlugin(): Promise<Plugin | undefined> {
async function getSentryViteSubPlugin(name: string): Promise<Plugin | undefined> {
const plugins = await makeCustomSentryVitePlugins({
authToken: 'token',
org: 'org',
project: 'project',
adapter: 'other',
});
return plugins.find(plugin => plugin.name === 'sentry-upload-sveltekit-source-maps');

return plugins.find(plugin => plugin.name === name);
}

describe('makeCustomSentryVitePlugin()', () => {
it('returns the custom sentry source maps plugin', async () => {
const plugin = await getCustomSentryViteUploadSourcemapsPlugin();
expect(plugin?.name).toEqual('sentry-upload-sveltekit-source-maps');
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');

expect(plugin?.name).toEqual('sentry-sveltekit-debug-id-upload-plugin');
expect(plugin?.apply).toEqual('build');
expect(plugin?.enforce).toEqual('post');

Expand All @@ -58,9 +74,9 @@ describe('makeCustomSentryVitePlugin()', () => {
expect(plugin?.writeBundle).toBeUndefined();
});

describe('Custom sentry vite plugin', () => {
describe('Custom debug id source maps plugin plugin', () => {
it('enables source map generation', async () => {
const plugin = await getCustomSentryViteUploadSourcemapsPlugin();
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
// @ts-expect-error this function exists!
const sentrifiedConfig = plugin.config({ build: { foo: {} }, test: {} });
expect(sentrifiedConfig).toEqual({
Expand All @@ -73,7 +89,7 @@ describe('makeCustomSentryVitePlugin()', () => {
});

it('injects the output dir into the server hooks file', async () => {
const plugin = await getCustomSentryViteUploadSourcemapsPlugin();
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
// @ts-expect-error this function exists!
const transformOutput = await plugin.transform('foo', '/src/hooks.server.ts');
const transformedCode = transformOutput.code;
Expand All @@ -84,34 +100,34 @@ describe('makeCustomSentryVitePlugin()', () => {
});

it('uploads source maps during the SSR build', async () => {
const plugin = await getCustomSentryViteUploadSourcemapsPlugin();
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
// @ts-expect-error this function exists!
plugin.configResolved({ build: { ssr: true } });
// @ts-expect-error this function exists!
await plugin.closeBundle();
expect(mockedSentryVitePlugin.writeBundle).toHaveBeenCalledTimes(1);
expect(mockedViteDebugIdUploadPlugin.writeBundle).toHaveBeenCalledTimes(1);
});

it("doesn't upload source maps during the non-SSR builds", async () => {
const plugin = await getCustomSentryViteUploadSourcemapsPlugin();
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');

// @ts-expect-error this function exists!
plugin.configResolved({ build: { ssr: false } });
// @ts-expect-error this function exists!
await plugin.closeBundle();
expect(mockedSentryVitePlugin.writeBundle).not.toHaveBeenCalled();
expect(mockedViteDebugIdUploadPlugin.writeBundle).not.toHaveBeenCalled();
});
});

it('catches errors while uploading source maps', async () => {
mockedSentryVitePlugin.writeBundle.mockImplementationOnce(() => {
mockedViteDebugIdUploadPlugin.writeBundle.mockImplementationOnce(() => {
throw new Error('test error');
});

const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {});
const consoleLogSpy = vi.spyOn(console, 'log').mockImplementationOnce(() => {});

const plugin = await getCustomSentryViteUploadSourcemapsPlugin();
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');

// @ts-expect-error this function exists!
expect(plugin.closeBundle).not.toThrow();
Expand All @@ -124,4 +140,100 @@ describe('makeCustomSentryVitePlugin()', () => {
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to upload source maps'));
expect(consoleLogSpy).toHaveBeenCalled();
});

describe('Custom release management plugin', () => {
it('has the expected hooks and properties', async () => {
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin');

expect(plugin).toEqual({
name: 'sentry-sveltekit-release-management-plugin',
apply: 'build',
enforce: 'post',
closeBundle: expect.any(Function),
});
});

it('calls the original release management plugin to start the release creation pipeline', async () => {
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin');
// @ts-expect-error this function exists!
await plugin.closeBundle();
expect(mockedViteReleaseManagementPlugin.writeBundle).toHaveBeenCalledTimes(1);
});

it('catches errors during release creation', async () => {
mockedViteReleaseManagementPlugin.writeBundle.mockImplementationOnce(() => {
throw new Error('test error');
});

const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {});

const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin');

// @ts-expect-error this function exists!
expect(plugin.closeBundle).not.toThrow();

// @ts-expect-error this function exists!
await plugin.closeBundle();

expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Failed to upload release data'),
expect.any(Error),
);
});

it('also works correctly if the original release management plugin has its old name', async () => {
const currentName = mockedViteReleaseManagementPlugin.name;
mockedViteReleaseManagementPlugin.name = 'sentry-debug-id-upload-plugin';

const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin');

// @ts-expect-error this function exists!
await plugin.closeBundle();

expect(mockedViteReleaseManagementPlugin.writeBundle).toHaveBeenCalledTimes(1);

mockedViteReleaseManagementPlugin.name = currentName;
});
});

describe('Custom file deletion plugin', () => {
it('has the expected hooks and properties', async () => {
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-file-deletion-plugin');

expect(plugin).toEqual({
name: 'sentry-sveltekit-file-deletion-plugin',
apply: 'build',
enforce: 'post',
closeBundle: expect.any(Function),
});
});

it('calls the original file deletion plugin to delete files', async () => {
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-file-deletion-plugin');
// @ts-expect-error this function exists!
await plugin.closeBundle();
expect(mockedFileDeletionPlugin.writeBundle).toHaveBeenCalledTimes(1);
});

it('catches errors during file deletion', async () => {
mockedFileDeletionPlugin.writeBundle.mockImplementationOnce(() => {
throw new Error('test error');
});

const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {});

const plugin = await getSentryViteSubPlugin('sentry-sveltekit-file-deletion-plugin');

// @ts-expect-error this function exists!
expect(plugin.closeBundle).not.toThrow();

// @ts-expect-error this function exists!
await plugin.closeBundle();

expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Failed to delete source maps'),
expect.any(Error),
);
});
});
});
Loading