review comments: add test

Author: s1gr1d

packages/nuxt/src/common/types.ts

@@ -129,11 +129,12 @@ export type SentryNuxtModuleOptions = {
   dynamicImportForServerEntry?: boolean;
 
   /**
-   * The `asyncFunctionReExports` option is only relevant when `dynamicImportForServerEntry: true` (default value).
+   * By default—unless you configure `dynamicImportForServerEntry: false`—the SDK will try to wrap your application entrypoint
+   * with a dynamic `import()` to ensure all dependencies can be properly instrumented.
    *
-   * As the server entry file is wrapped with a dynamic `import()`, previous async function exports need to be re-exported.
-   * The SDK detects and re-exports those exports (mostly serverless functions). This is why they are re-exported as async functions.
-   * In case you have a custom setup and your server exports other async functions, you can override the default array with this option.
+   * By default, the SDK will wrap the default export as well as a `handler` or `server` export from the entrypoint.
+   * If your application has a different main export that is used to run the application, you can overwrite this by providing an array of export names to wrap.
+   * Any wrapped export is expected to be an async function.
    *
    * @default ['default', 'handler', 'server']
    */

packages/nuxt/src/module.ts

@@ -21,9 +21,7 @@ export default defineNuxtModule<ModuleOptions>({
     const moduleOptions = {
       ...moduleOptionsParam,
       dynamicImportForServerEntry: moduleOptionsParam.dynamicImportForServerEntry !== false, // default: true
-      asyncFunctionReExports: moduleOptionsParam.asyncFunctionReExports
-        ? moduleOptionsParam.asyncFunctionReExports
-        : ['default', 'handler', 'server'],
+      asyncFunctionReExports: moduleOptionsParam.asyncFunctionReExports || ['default', 'handler', 'server'],
     };
 
     const moduleDirResolver = createResolver(import.meta.url);

packages/nuxt/src/vite/addServerConfig.ts

@@ -1,7 +1,7 @@
 import * as fs from 'fs';
 import { createResolver } from '@nuxt/kit';
 import type { Nuxt } from '@nuxt/schema';
-import { consoleSandbox, flatten } from '@sentry/utils';
+import { consoleSandbox } from '@sentry/utils';
 import type { Nitro } from 'nitropack';
 import type { InputPluginOption } from 'rollup';
 import type { SentryNuxtModuleOptions } from '../common/types';
@@ -10,6 +10,7 @@ import {
   SENTRY_FUNCTIONS_REEXPORT,
   SENTRY_WRAPPED_ENTRY,
   constructFunctionReExport,
+  constructFunctionsReExportQuery,
   removeSentryQueryFromPath,
 } from './utils';
 
@@ -99,7 +100,6 @@ export function addDynamicImportEntryFileWrapper(
   }
 
   nitro.options.rollupConfig.plugins.push(
-    // @ts-expect-error - This is the correct type, but it shows an error because of two different definitions
     wrapEntryWithDynamicImport({
       resolvedSentryConfigPath: createResolver(nitro.options.srcDir).resolve(`/${serverConfigFile}`),
       asyncFunctionReExports: moduleOptions.asyncFunctionReExports,
@@ -142,28 +142,13 @@ function wrapEntryWithDynamicImport({
 
         moduleInfo.moduleSideEffects = true;
 
-        // `exportedBindings` can look like this:  `{ '.': [ 'handler' ] }` or `{ '.': [], './firebase-gen-1.mjs': [ 'server' ] }`
-        // The key `.` refers to exports within the current file, while other keys show from where exports were imported first.
-        const functionsToExport = flatten(Object.values(moduleInfo.exportedBindings || {})).filter(functionName =>
-          asyncFunctionReExports.includes(functionName),
-        );
-
-        if (debug && functionsToExport.length === 0) {
-          consoleSandbox(() =>
-            // eslint-disable-next-line no-console
-            console.warn(
-              "[Sentry] No functions found for re-export. In case your server needs to export async functions other than `handler` or  `server`, consider adding the name(s) to Sentry's build options `sentry.asyncFunctionReExports` in your `nuxt.config.ts`.",
-            ),
-          );
-        }
-
         // The enclosing `if` already checks for the suffix in `source`, but a check in `resolution.id` is needed as well to prevent multiple attachment of the suffix
         return resolution.id.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)
           ? resolution.id
           : resolution.id
               // Concatenates the query params to mark the file (also attaches names of re-exports - this is needed for serverless functions to re-export the handler)
               .concat(SENTRY_WRAPPED_ENTRY)
-              .concat(functionsToExport?.length ? SENTRY_FUNCTIONS_REEXPORT.concat(functionsToExport.join(',')) : '')
+              .concat(constructFunctionsReExportQuery(moduleInfo.exportedBindings, asyncFunctionReExports, debug))
               .concat(QUERY_END_INDICATOR);
       }
       return null;

packages/nuxt/src/vite/utils.ts

@@ -1,5 +1,6 @@
 import * as fs from 'fs';
 import * as path from 'path';
+import { consoleSandbox, flatten } from '@sentry/utils';
 
 /**
  *  Find the default SDK init file for the given type (client or server).
@@ -63,20 +64,47 @@ export function extractFunctionReexportQueryParameters(query: string): string[]
     : [];
 }
 
+/**
+ *  Constructs a comma-separated string with all functions that need to be re-exported later from the server entry.
+ *  It uses Rollup's `exportedBindings` to determine the functions to re-export
+ */
+export function constructFunctionsReExportQuery(
+  exportedBindings: Record<string, string[]> | null,
+  asyncFunctionReExports: string[],
+  debug?: boolean,
+): string {
+  // `exportedBindings` can look like this:  `{ '.': [ 'handler' ] }` or `{ '.': [], './firebase-gen-1.mjs': [ 'server' ] }`
+  // The key `.` refers to exports within the current file, while other keys show from where exports were imported first.
+  const functionsToExport = flatten(Object.values(exportedBindings || {})).filter(functionName =>
+    asyncFunctionReExports.includes(functionName),
+  );
+
+  if (debug && functionsToExport.length === 0) {
+    consoleSandbox(() =>
+      // eslint-disable-next-line no-console
+      console.warn(
+        "[Sentry] No functions found for re-export. In case your server needs to export async functions other than `handler` or  `server`, consider adding the name(s) to Sentry's build options `sentry.asyncFunctionReExports` in your `nuxt.config.ts`.",
+      ),
+    );
+  }
+
+  return functionsToExport?.length ? SENTRY_FUNCTIONS_REEXPORT.concat(functionsToExport.join(',')) : '';
+}
+
 /**
  * Constructs a code snippet with function reexports (can be used in Rollup plugins)
  */
 export function constructFunctionReExport(pathWithQuery: string, entryId: string): string {
   const functionNames = extractFunctionReexportQueryParameters(pathWithQuery);
 
   return functionNames.reduce(
-    (functionsCode, currFunctionName, idx) =>
+    (functionsCode, currFunctionName) =>
       functionsCode.concat(
-        `async function reExport${idx}(...args) {\n` +
+        `async function reExport${currFunctionName.toUpperCase()}(...args) {\n` +
           `  const res = await import(${JSON.stringify(entryId)});\n` +
           `  return res.${currFunctionName}.call(this, ...args);\n` +
           '}\n' +
-          `export { reExport${idx} as ${currFunctionName} };\n`,
+          `export { reExport${currFunctionName.toUpperCase()} as ${currFunctionName} };\n`,
       ),
     '',
   );

packages/nuxt/test/vite/utils.test.ts

@@ -5,6 +5,7 @@ import {
   SENTRY_FUNCTIONS_REEXPORT,
   SENTRY_WRAPPED_ENTRY,
   constructFunctionReExport,
+  constructFunctionsReExportQuery,
   extractFunctionReexportQueryParameters,
   findDefaultSdkInitFile,
   removeSentryQueryFromPath,
@@ -102,6 +103,38 @@ describe('extractFunctionReexportQueryParameters', () => {
   });
 });
 
+describe('constructFunctionsReExportQuery', () => {
+  it.each([
+    [{ '.': ['handler'] }, ['handler'], '?sentry-query-functions-reexport=handler'],
+    [{ '.': ['handler'], './module': ['server'] }, [], ''],
+    [{ '.': ['handler'], './module': ['server'] }, ['server'], '?sentry-query-functions-reexport=server'],
+    [{ '.': ['handler', 'otherFunction'] }, ['handler'], '?sentry-query-functions-reexport=handler'],
+    [{ '.': ['handler', 'otherFn'] }, ['handler', 'otherFn'], '?sentry-query-functions-reexport=handler,otherFn'],
+    [{ '.': ['bar'], './module': ['foo'] }, ['bar', 'foo'], '?sentry-query-functions-reexport=bar,foo'],
+  ])(
+    'constructs re-export query for exportedBindings: %j and asyncFunctionReExports: %j',
+    (exportedBindings, asyncFunctionReExports, expected) => {
+      const result = constructFunctionsReExportQuery(exportedBindings, asyncFunctionReExports);
+      expect(result).toBe(expected);
+    },
+  );
+
+  it('logs a warning if no functions are found for re-export and debug is true', () => {
+    const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const exportedBindings = { '.': ['handler'] };
+    const asyncFunctionReExports = ['nonExistentFunction'];
+    const debug = true;
+
+    const result = constructFunctionsReExportQuery(exportedBindings, asyncFunctionReExports, debug);
+    expect(result).toBe('');
+    expect(consoleWarnSpy).toHaveBeenCalledWith(
+      "[Sentry] No functions found for re-export. In case your server needs to export async functions other than `handler` or  `server`, consider adding the name(s) to Sentry's build options `sentry.asyncFunctionReExports` in your `nuxt.config.ts`.",
+    );
+
+    consoleWarnSpy.mockRestore();
+  });
+});
+
 describe('constructFunctionReExport', () => {
   it('constructs re-export code for given query parameters and entry ID', () => {
     const query = `${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}}`;
@@ -111,16 +144,16 @@ describe('constructFunctionReExport', () => {
     const result2 = constructFunctionReExport(query2, entryId);
 
     const expected = `
-async function reExport0(...args) {
+async function reExportFOO(...args) {
   const res = await import("./module");
   return res.foo.call(this, ...args);
 }
-export { reExport0 as foo };
-async function reExport1(...args) {
+export { reExportFOO as foo };
+async function reExportBAR(...args) {
   const res = await import("./module");
   return res.bar.call(this, ...args);
 }
-export { reExport1 as bar };
+export { reExportBAR as bar };
 `;
     expect(result.trim()).toBe(expected.trim());
     expect(result2.trim()).toBe(expected.trim());
@@ -132,11 +165,26 @@ export { reExport1 as bar };
     const result = constructFunctionReExport(query, entryId);
 
     const expected = `
-async function reExport0(...args) {
+async function reExportDEFAULT(...args) {
+  const res = await import("./index");
+  return res.default.call(this, ...args);
+}
+export { reExportDEFAULT as default };
+`;
+    expect(result.trim()).toBe(expected.trim());
+  });
+
+  it('constructs re-export code for a "default" query parameters and entry ID', () => {
+    const query = `${SENTRY_FUNCTIONS_REEXPORT}default${QUERY_END_INDICATOR}}`;
+    const entryId = './index';
+    const result = constructFunctionReExport(query, entryId);
+
+    const expected = `
+async function reExportDEFAULT(...args) {
   const res = await import("./index");
   return res.default.call(this, ...args);
 }
-export { reExport0 as default };
+export { reExportDEFAULT as default };
 `;
     expect(result.trim()).toBe(expected.trim());
   });