Skip to content

Commit

Permalink
fix(sveltekit): Ensure source maps deletion is called after source ma…
Browse files Browse the repository at this point in the history 
…ps upload
  • Loading branch information
@Lms24
Lms24 committed Jan 8, 2025
1 parent e8e84ea commit df67de1
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 25 deletions.
89 changes: 85 additions & 4 deletions packages/sveltekit/src/vite/sourceMaps.ts
View file
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
View file
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
View file
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),
);
});
});
});

0 comments on commit df67de1

Please # to comment.