From e3dda4ce6435f55e364da85016e55c162f6885a9 Mon Sep 17 00:00:00 2001
From: Lukas Stracke <lukas.stracke@sentry.io>
Date: Tue, 29 Aug 2023 10:28:17 +0200
Subject: [PATCH] fix(sveltekit): Ensure target file exists before applying
 auto instrumentation (#8881)

In our auto instrumentation Vite plugin for SvelteKit, we read
`+(page|layout)(.server).(js|ts)` files' code to determine if we should
add our wrapper to the file or not. We previously didn't check for a
file id's existence before reading the file if the id matched that
certain pattern, wrongly assuming that these ids would always map to
actually existing files.

It seems like Vite plugins such as Houdini's plugin add file ids to the
build for files that actually don't exist (#8846, #8854) . When our
plugin's `load` hook was called for such an id, it then tried to access
and read the file which caused a build error.

This patch now adds a file existence guard to ensure we simply no-op for
these files.
---
 packages/sveltekit/src/vite/autoInstrument.ts  | 18 +++++++++++++++---
 .../sveltekit/test/vite/autoInstrument.test.ts | 17 ++++++++++++++---
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts
index cabfc743db0c..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));
 
@@ -90,7 +90,19 @@ 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<boolean> {
+  // 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 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);
   });
 });