From 8c9ae3306b84a77803b6a4753c775c5e8d5f3ea8 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Aug 2023 14:39:58 +0200 Subject: [PATCH 1/2] fix(sveltekit): Ensure file exists before applying auto instrumentation --- packages/sveltekit/src/vite/autoInstrument.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts index cabfc743db0c..0783d1e840a8 100644 --- a/packages/sveltekit/src/vite/autoInstrument.ts +++ b/packages/sveltekit/src/vite/autoInstrument.ts @@ -90,6 +90,12 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio * @returns `true` if we can wrap the given file, `false` otherwise */ export async function canWrapLoad(id: string, debug: boolean): Promise { + // Some 3rd party plugins add ids to the build that actually don't exist. + // We need to check for that here, otherwise users get get a build errirs. + if (!fs.existsSync(id)) { + return false; + } + const code = (await fs.promises.readFile(id, 'utf8')).toString(); const mod = parseModule(code); From eed44b57f93b4c16c9ed5312e8db484b588373e3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 28 Aug 2023 16:20:02 +0200 Subject: [PATCH 2/2] adjust and fix tests --- packages/sveltekit/src/vite/autoInstrument.ts | 14 ++++++++++---- .../sveltekit/test/vite/autoInstrument.test.ts | 17 ++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts index 0783d1e840a8..07e8b7646124 100644 --- a/packages/sveltekit/src/vite/autoInstrument.ts +++ b/packages/sveltekit/src/vite/autoInstrument.ts @@ -40,7 +40,7 @@ type AutoInstrumentPluginOptions = AutoInstrumentSelection & { * @returns the plugin */ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptions): Plugin { - const { load: shouldWrapLoad, serverLoad: shouldWrapServerLoad, debug } = options; + const { load: wrapLoadEnabled, serverLoad: wrapServerLoadEnabled, debug } = options; return { name: 'sentry-auto-instrumentation', @@ -49,7 +49,7 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio async load(id) { const applyUniversalLoadWrapper = - shouldWrapLoad && + wrapLoadEnabled && /^\+(page|layout)\.(js|ts|mjs|mts)$/.test(path.basename(id)) && (await canWrapLoad(id, debug)); @@ -60,7 +60,7 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio } const applyServerLoadWrapper = - shouldWrapServerLoad && + wrapServerLoadEnabled && /^\+(page|layout)\.server\.(js|ts|mjs|mts)$/.test(path.basename(id)) && (await canWrapLoad(id, debug)); @@ -91,12 +91,18 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio */ export async function canWrapLoad(id: string, debug: boolean): Promise { // Some 3rd party plugins add ids to the build that actually don't exist. - // We need to check for that here, otherwise users get get a build errirs. + // We need to check for that here, otherwise users get get a build errors. if (!fs.existsSync(id)) { + debug && + // eslint-disable-next-line no-console + console.log( + `Skipping wrapping ${id} because it doesn't exist. A 3rd party plugin might have added this as a virtual file to the build`, + ); return false; } const code = (await fs.promises.readFile(id, 'utf8')).toString(); + const mod = parseModule(code); const program = mod.$ast.type === 'Program' && mod.$ast; diff --git a/packages/sveltekit/test/vite/autoInstrument.test.ts b/packages/sveltekit/test/vite/autoInstrument.test.ts index 954138f017bf..13ee56eef3c6 100644 --- a/packages/sveltekit/test/vite/autoInstrument.test.ts +++ b/packages/sveltekit/test/vite/autoInstrument.test.ts @@ -21,6 +21,12 @@ vi.mock('fs', async () => { return fileContent || DEFAULT_CONTENT; }), }, + existsSync: vi.fn().mockImplementation(id => { + if (id === '+page.virtual.ts') { + return false; + } + return true; + }), }; }); @@ -198,15 +204,20 @@ describe('canWrapLoad', () => { 'export const loadNotLoad = () => {}; export const prerender = true;', 'export function aload(){}; export const prerender = true;', 'export function loader(){}; export const prerender = true;', - 'let loademe = false; export {loadme}', + 'let loadme = false; export {loadme}', 'const a = {load: true}; export {a}', 'if (s === "load") {}', 'const a = load ? load : false', '// const load = () => {}', '/* export const load = () => {} */ export const prerender = true;', '/* export const notLoad = () => { const a = getSomething() as load; } */ export const prerender = true;', - ])('returns `false` if no load declaration exists', async (_, code) => { + ])('returns `false` if no load declaration exists', async code => { fileContent = code; - expect(await canWrapLoad('+page.ts', false)).toEqual(true); + expect(await canWrapLoad('+page.ts', false)).toEqual(false); + }); + + it("returns `false` if the passed file id doesn't exist", async () => { + fileContent = DEFAULT_CONTENT; + expect(await canWrapLoad('+page.virtual.ts', false)).toEqual(false); }); });