fix(nextjs): Fix serverside transaction names on Windows (#9526)

getsentry · Nov 10, 2023 · 1d50eef · 1d50eef
1 parent 82e87dc
commit 1d50eef
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 17 deletions.
diff --git a/packages/nextjs/src/config/loaders/types.ts b/packages/nextjs/src/config/loaders/types.ts
@@ -8,13 +8,6 @@ export type LoaderThis<Options> = {
    */
   resourcePath: string;
 
-  /**
-   * Query at the end of resolved file name ("../some-folder/some-module?foobar" -> resourceQuery: "?foobar")
-   *
-   * https://webpack.js.org/api/loaders/#thisresourcequery
-   */
-  resourceQuery: string;
-
   /**
    * Function to add outside file used by loader to `watch` process
    *

diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts
@@ -40,7 +40,7 @@ const serverComponentWrapperTemplateCode = fs.readFileSync(serverComponentWrappe
 const routeHandlerWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'routeHandlerWrapperTemplate.js');
 const routeHandlerWrapperTemplateCode = fs.readFileSync(routeHandlerWrapperTemplatePath, { encoding: 'utf8' });
 
-type LoaderOptions = {
+export type WrappingLoaderOptions = {
   pagesDir: string;
   appDir: string;
   pageExtensionRegex: string;
@@ -58,7 +58,7 @@ type LoaderOptions = {
  */
 // eslint-disable-next-line complexity
 export default function wrappingLoader(
-  this: LoaderThis<LoaderOptions>,
+  this: LoaderThis<WrappingLoaderOptions>,
   userCode: string,
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   userModuleSourceMap: any,
@@ -102,12 +102,11 @@ export default function wrappingLoader(
     }
   } else if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') {
     // Get the parameterized route name from this page's filepath
-    const parameterizedPagesRoute = path.posix
-      .normalize(
-        path
-          // Get the path of the file insde of the pages directory
-          .relative(pagesDir, this.resourcePath),
-      )
+    const parameterizedPagesRoute = path
+      // Get the path of the file insde of the pages directory
+      .relative(pagesDir, this.resourcePath)
+      // Replace all backslashes with forward slashes (windows)
+      .replace(/\\/g, '/')
       // Add a slash at the beginning
       .replace(/(.*)/, '/$1')
       // Pull off the file extension
@@ -139,8 +138,11 @@ export default function wrappingLoader(
     templateCode = templateCode.replace(/__ROUTE__/g, parameterizedPagesRoute.replace(/\\/g, '\\\\'));
   } else if (wrappingTargetKind === 'server-component' || wrappingTargetKind === 'route-handler') {
     // Get the parameterized route name from this page's filepath
-    const parameterizedPagesRoute = path.posix
-      .normalize(path.relative(appDir, this.resourcePath))
+    const parameterizedPagesRoute = path
+      // Get the path of the file insde of the app directory
+      .relative(appDir, this.resourcePath)
+      // Replace all backslashes with forward slashes (windows)
+      .replace(/\\/g, '/')
       // Add a slash at the beginning
       .replace(/(.*)/, '/$1')
       // Pull off the file name

diff --git a/packages/nextjs/test/config/wrappingLoader.test.ts b/packages/nextjs/test/config/wrappingLoader.test.ts
@@ -0,0 +1,105 @@
+import * as fs from 'fs';
+import * as path from 'path';
+
+const originalReadfileSync = fs.readFileSync;
+
+jest.spyOn(fs, 'readFileSync').mockImplementation((filePath, options) => {
+  if (filePath.toString().endsWith('/config/templates/apiWrapperTemplate.js')) {
+    return originalReadfileSync(
+      path.join(__dirname, '../../build/cjs/config/templates/apiWrapperTemplate.js'),
+      options,
+    );
+  }
+
+  if (filePath.toString().endsWith('/config/templates/pageWrapperTemplate.js')) {
+    return originalReadfileSync(
+      path.join(__dirname, '../../build/cjs/config/templates/pageWrapperTemplate.js'),
+      options,
+    );
+  }
+
+  if (filePath.toString().endsWith('/config/templates/middlewareWrapperTemplate.js')) {
+    return originalReadfileSync(
+      path.join(__dirname, '../../build/cjs/config/templates/middlewareWrapperTemplate.js'),
+      options,
+    );
+  }
+
+  if (filePath.toString().endsWith('/config/templates/sentryInitWrapperTemplate.js')) {
+    return originalReadfileSync(
+      path.join(__dirname, '../../build/cjs/config/templates/sentryInitWrapperTemplate.js'),
+      options,
+    );
+  }
+
+  if (filePath.toString().endsWith('/config/templates/serverComponentWrapperTemplate.js')) {
+    return originalReadfileSync(
+      path.join(__dirname, '../../build/cjs/config/templates/serverComponentWrapperTemplate.js'),
+      options,
+    );
+  }
+
+  if (filePath.toString().endsWith('/config/templates/routeHandlerWrapperTemplate.js')) {
+    return originalReadfileSync(
+      path.join(__dirname, '../../build/cjs/config/templates/routeHandlerWrapperTemplate.js'),
+      options,
+    );
+  }
+
+  return originalReadfileSync(filePath, options);
+});
+
+import type { LoaderThis } from '../../src/config/loaders/types';
+import type { WrappingLoaderOptions } from '../../src/config/loaders/wrappingLoader';
+import wrappingLoader from '../../src/config/loaders/wrappingLoader';
+
+const DEFAULT_PAGE_EXTENSION_REGEX = ['tsx', 'ts', 'jsx', 'js'].join('|');
+
+const defaultLoaderThis = {
+  addDependency: () => undefined,
+  async: () => undefined,
+  cacheable: () => undefined,
+};
+
+describe('wrappingLoader', () => {
+  it('should correctly wrap API routes on unix', async () => {
+    const callback = jest.fn();
+
+    const userCode = `
+      export default function handler(req, res) {
+        res.json({ foo: "bar" });
+      }
+    `;
+    const userCodeSourceMap = undefined;
+
+    const loaderPromise = new Promise<void>(resolve => {
+      const loaderThis = {
+        ...defaultLoaderThis,
+        resourcePath: '/my/pages/my/route.ts',
+        callback: callback.mockImplementation(() => {
+          resolve();
+        }),
+        getOptions() {
+          return {
+            pagesDir: '/my/pages',
+            appDir: '/my/app',
+            pageExtensionRegex: DEFAULT_PAGE_EXTENSION_REGEX,
+            excludeServerRoutes: [],
+            wrappingTargetKind: 'api-route',
+            sentryConfigFilePath: '/my/sentry.server.config.ts',
+            vercelCronsConfig: undefined,
+            nextjsRequestAsyncStorageModulePath: '/my/request-async-storage.js',
+          };
+        },
+      } satisfies LoaderThis<WrappingLoaderOptions>;
+
+      wrappingLoader.call(loaderThis, userCode, userCodeSourceMap);
+    });
+
+    await loaderPromise;
+
+    expect(callback).toHaveBeenCalledWith(null, expect.stringContaining("'/my/route'"), expect.anything());
+  });
+
+  it.todo('should correctly wrap API routes on unix');
+});