From 200ba3a703d53926ff30b377a14ad23374874707 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 11 Oct 2024 14:44:36 +0200 Subject: [PATCH 1/5] feat(nuxt): Make dynamic `import()` wrapping default --- packages/nuxt/src/common/types.ts | 12 +++--- packages/nuxt/src/module.ts | 41 ++++++++++++------- packages/nuxt/src/vite/addServerConfig.ts | 48 ----------------------- 3 files changed, 32 insertions(+), 69 deletions(-) diff --git a/packages/nuxt/src/common/types.ts b/packages/nuxt/src/common/types.ts index bcc14ad1d307..c96ef20a63b6 100644 --- a/packages/nuxt/src/common/types.ts +++ b/packages/nuxt/src/common/types.ts @@ -103,16 +103,16 @@ export type SentryNuxtModuleOptions = { debug?: boolean; /** - * Enabling basic server tracing can be used for environments where modifying the node option `--import` is not possible. - * However, enabling this option only supports limited tracing instrumentation. Only http traces will be collected (but no database-specific traces etc.). + * If this option is `false`, the Sentry SDK won't wrap the server entry file with `import()`. Not wrapping the + * server entry file will disable Sentry on the server-side. When you set this option to `true`, make sure + * to add the sentry server config with the node `--import` CLI flag to enable Sentry on the server-side. * - * If this option is `true`, the Sentry SDK will import the Sentry server config at the top of the server entry file to load the SDK on the server. - * - * **DO NOT** enable this option if you've already added the node option `--import` in your node start script. This would initialize Sentry twice on the server-side and leads to unexpected issues. + * **DO NOT** add the node CLI flag `--import` in your node start script, when `disableDynamicImportWrapping` is set to `false`. + * This would initialize Sentry twice on the server-side and this leads to unexpected issues. * * @default false */ - experimental_basicServerTracing?: boolean; + disableDynamicImportWrapping?: boolean; /** * Options to be passed directly to the Sentry Rollup Plugin (`@sentry/rollup-plugin`) and Sentry Vite Plugin (`@sentry/vite-plugin`) that ship with the Sentry Nuxt SDK. diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index c74fe32b93fe..b273aebef56c 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -2,7 +2,7 @@ import * as path from 'path'; import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit'; import { consoleSandbox } from '@sentry/utils'; import type { SentryNuxtModuleOptions } from './common/types'; -import { addSentryTopImport, addServerConfigToBuild } from './vite/addServerConfig'; +import { addDynamicImportEntryFileWrapper, addServerConfigToBuild } from './vite/addServerConfig'; import { setupSourceMaps } from './vite/sourceMaps'; import { findDefaultSdkInitFile } from './vite/utils'; @@ -48,17 +48,19 @@ export default defineNuxtModule({ const serverConfigFile = findDefaultSdkInitFile('server'); if (serverConfigFile) { - // Inject the server-side Sentry config file with a side effect import - addPluginTemplate({ - mode: 'server', - filename: 'sentry-server-config.mjs', - getContents: () => - `import "${buildDirResolver.resolve(`/${serverConfigFile}`)}"\n` + - 'import { defineNuxtPlugin } from "#imports"\n' + - 'export default defineNuxtPlugin(() => {})', - }); + if (moduleOptions.disableDynamicImportWrapping) { + // Inject the server-side Sentry config file with a side effect import + addPluginTemplate({ + mode: 'server', + filename: 'sentry-server-config.mjs', + getContents: () => + `import "${buildDirResolver.resolve(`/${serverConfigFile}`)}"\n` + + 'import { defineNuxtPlugin } from "#imports"\n' + + 'export default defineNuxtPlugin(() => {})', + }); - addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server')); + addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server')); + } } if (clientConfigFile || serverConfigFile) { @@ -67,11 +69,9 @@ export default defineNuxtModule({ nuxt.hooks.hook('nitro:init', nitro => { if (serverConfigFile && serverConfigFile.includes('.server.config')) { - addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); + if (moduleOptions.disableDynamicImportWrapping) { + addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); - if (moduleOptions.experimental_basicServerTracing) { - addSentryTopImport(moduleOptions, nitro); - } else { if (moduleOptions.debug) { const serverDirResolver = createResolver(nitro.options.output.serverDir); const serverConfigPath = serverDirResolver.resolve('sentry.server.config.mjs'); @@ -86,6 +86,17 @@ export default defineNuxtModule({ ); }); } + } else { + addDynamicImportEntryFileWrapper(nitro, serverConfigFile); + + if (moduleOptions.debug) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + '[Sentry] Wrapping the server entry file with a dynamic `import()`, so Sentry can be preloaded before the server initializes.', + ); + }); + } } } }); diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index 5a5d2e5bd627..fa784f93b677 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -74,54 +74,6 @@ export function addServerConfigToBuild( }); } -/** - * Adds the Sentry server config import at the top of the server entry file to load the SDK on the server. - * This is necessary for environments where modifying the node option `--import` is not possible. - * However, only limited tracing instrumentation is supported when doing this. - */ -export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro: Nitro): void { - nitro.hooks.hook('close', () => { - // other presets ('node-server' or 'vercel') have an index.mjs - const presetsWithServerFile = ['netlify']; - const entryFileName = - typeof nitro.options.rollupConfig?.output.entryFileNames === 'string' - ? nitro.options.rollupConfig?.output.entryFileNames - : presetsWithServerFile.includes(nitro.options.preset) - ? 'server.mjs' - : 'index.mjs'; - - const serverDirResolver = createResolver(nitro.options.output.serverDir); - const entryFilePath = serverDirResolver.resolve(entryFileName); - - try { - fs.readFile(entryFilePath, 'utf8', (err, data) => { - const updatedContent = `import './${SERVER_CONFIG_FILENAME}.mjs';\n${data}`; - - fs.writeFile(entryFilePath, updatedContent, 'utf8', () => { - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Successfully added the Sentry import to the server entry file "\`${entryFilePath}\`"`, - ); - }); - } - }); - }); - } catch (err) { - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] An error occurred when trying to add the Sentry import to the server entry file "\`${entryFilePath}\`":`, - err, - ); - }); - } - } - }); -} - /** * This function modifies the Rollup configuration to include a plugin that wraps the entry file with a dynamic import (`import()`) * and adds the Sentry server config with the static `import` declaration. From 4ffcaa8bf3f51196f9b80e3e3f0bde2069e31b78 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 11 Oct 2024 15:48:21 +0200 Subject: [PATCH 2/5] delete --import --- dev-packages/e2e-tests/test-applications/nuxt-3/package.json | 2 +- dev-packages/e2e-tests/test-applications/nuxt-4/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nuxt-3/package.json b/dev-packages/e2e-tests/test-applications/nuxt-3/package.json index 079beb8bfc86..6c8eb1fcdd95 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-3/package.json +++ b/dev-packages/e2e-tests/test-applications/nuxt-3/package.json @@ -7,7 +7,7 @@ "dev": "nuxt dev", "generate": "nuxt generate", "preview": "nuxt preview", - "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs", + "start": "node .output/server/index.mjs", "clean": "npx nuxi cleanup", "test": "playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4/package.json b/dev-packages/e2e-tests/test-applications/nuxt-4/package.json index 5e7bcd57af0e..db56273a7493 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4/package.json +++ b/dev-packages/e2e-tests/test-applications/nuxt-4/package.json @@ -7,7 +7,7 @@ "dev": "nuxt dev", "generate": "nuxt generate", "preview": "nuxt preview", - "start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs", + "start": "node .output/server/index.mjs", "clean": "npx nuxi cleanup", "test": "playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", From 3c1829f19c1df2d9df6e2e14ab1c7fa261d6c6bc Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Mon, 14 Oct 2024 16:56:51 +0200 Subject: [PATCH 3/5] fix dev mode --- packages/nuxt/src/common/types.ts | 13 ++++++++----- packages/nuxt/src/module.ts | 11 ++++++++--- packages/nuxt/src/vite/addServerConfig.ts | 9 +++------ packages/nuxt/test/vite/utils.test.ts | 3 +++ 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/nuxt/src/common/types.ts b/packages/nuxt/src/common/types.ts index c96ef20a63b6..e8503d8e3dd6 100644 --- a/packages/nuxt/src/common/types.ts +++ b/packages/nuxt/src/common/types.ts @@ -103,16 +103,19 @@ export type SentryNuxtModuleOptions = { debug?: boolean; /** + * Wraps the server entry file with a dynamic `import()`. This will make it possible to preload Sentry and register + * necessary hooks before other code runs. (Node docs: https://nodejs.org/api/module.html#enabling) + * * If this option is `false`, the Sentry SDK won't wrap the server entry file with `import()`. Not wrapping the - * server entry file will disable Sentry on the server-side. When you set this option to `true`, make sure - * to add the sentry server config with the node `--import` CLI flag to enable Sentry on the server-side. + * server entry file will disable Sentry on the server-side. When you set this option to `false`, make sure + * to add the Sentry server config with the node `--import` CLI flag to enable Sentry on the server-side. * - * **DO NOT** add the node CLI flag `--import` in your node start script, when `disableDynamicImportWrapping` is set to `false`. + * **DO NOT** add the node CLI flag `--import` in your node start script, when `dynamicImportWrapping` is set to `true` (default). * This would initialize Sentry twice on the server-side and this leads to unexpected issues. * - * @default false + * @default true */ - disableDynamicImportWrapping?: boolean; + dynamicImportWrapping?: boolean; /** * Options to be passed directly to the Sentry Rollup Plugin (`@sentry/rollup-plugin`) and Sentry Vite Plugin (`@sentry/vite-plugin`) that ship with the Sentry Nuxt SDK. diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index b273aebef56c..579474e63fbb 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -17,7 +17,12 @@ export default defineNuxtModule({ }, }, defaults: {}, - setup(moduleOptions, nuxt) { + setup(moduleOptionsParam, nuxt) { + const moduleOptions = { + ...moduleOptionsParam, + dynamicImportWrapping: moduleOptionsParam.dynamicImportWrapping !== false, // default: true + }; + const moduleDirResolver = createResolver(import.meta.url); const buildDirResolver = createResolver(nuxt.options.buildDir); @@ -48,7 +53,7 @@ export default defineNuxtModule({ const serverConfigFile = findDefaultSdkInitFile('server'); if (serverConfigFile) { - if (moduleOptions.disableDynamicImportWrapping) { + if (moduleOptions.dynamicImportWrapping === false) { // Inject the server-side Sentry config file with a side effect import addPluginTemplate({ mode: 'server', @@ -69,7 +74,7 @@ export default defineNuxtModule({ nuxt.hooks.hook('nitro:init', nitro => { if (serverConfigFile && serverConfigFile.includes('.server.config')) { - if (moduleOptions.disableDynamicImportWrapping) { + if (moduleOptions.dynamicImportWrapping === false) { addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); if (moduleOptions.debug) { diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index fa784f93b677..4c12c6fd7dc2 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -139,11 +139,8 @@ function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPlug : 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( - exportedFunctions?.length - ? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')).concat(QUERY_END_INDICATOR) - : '', - ); + .concat(exportedFunctions?.length ? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')) : '') + .concat(QUERY_END_INDICATOR); } return null; }, @@ -163,7 +160,7 @@ function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPlug // `import()` can be used for any code that should be run after the hooks are registered (https://nodejs.org/api/module.html#enabling) `import(${JSON.stringify(entryId)});\n` + // By importing "import-in-the-middle/hook.mjs", we can make sure this file wil be included, as not all node builders are including files imported with `module.register()`. - "import 'import-in-the-middle/hook.mjs'\n" + + "import 'import-in-the-middle/hook.mjs';\n" + `${reExportedFunctions}\n` ); } diff --git a/packages/nuxt/test/vite/utils.test.ts b/packages/nuxt/test/vite/utils.test.ts index 0d7e91b8b83f..0d006378fc98 100644 --- a/packages/nuxt/test/vite/utils.test.ts +++ b/packages/nuxt/test/vite/utils.test.ts @@ -71,8 +71,11 @@ describe('findDefaultSdkInitFile', () => { describe('removeSentryQueryFromPath', () => { it('strips the Sentry query part from the path', () => { const url = `/example/path${SENTRY_WRAPPED_ENTRY}${SENTRY_FUNCTIONS_REEXPORT}foo,${QUERY_END_INDICATOR}`; + const url2 = `/example/path${SENTRY_WRAPPED_ENTRY}${QUERY_END_INDICATOR}`; const result = removeSentryQueryFromPath(url); + const result2 = removeSentryQueryFromPath(url2); expect(result).toBe('/example/path'); + expect(result2).toBe('/example/path'); }); it('returns the same path if the specific query part is not present', () => { From aa74914daa33792e71077b24fd247334d470e2c4 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:02:27 +0200 Subject: [PATCH 4/5] fix(nuxt): Re-export 'default' exports with rollup plugin (#13984) While the preset for Netlify exports the serverless handler function as `export { D as default }`, the Vercel preset exports this handler as `export { D as handler };`. This PR makes some adaptions to the code generation in the plugin to make it possible to re-export `default` functions. The previous snippet did not work as `default` is obviously not allowed as a function name. --- packages/nuxt/src/vite/utils.ts | 7 ++++--- packages/nuxt/test/vite/utils.test.ts | 26 ++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/nuxt/src/vite/utils.ts b/packages/nuxt/src/vite/utils.ts index 4d5bc080ba3a..1737a47e8062 100644 --- a/packages/nuxt/src/vite/utils.ts +++ b/packages/nuxt/src/vite/utils.ts @@ -57,7 +57,7 @@ export function extractFunctionReexportQueryParameters(query: string): string[] return match && match[1] ? match[1] .split(',') - .filter(param => param !== '' && param !== 'default') + .filter(param => param !== '') // Sanitize, as code could be injected with another rollup plugin .map((str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) : []; @@ -72,10 +72,11 @@ export function constructFunctionReExport(pathWithQuery: string, entryId: string return functionNames.reduce( (functionsCode, currFunctionName) => functionsCode.concat( - `export async function ${currFunctionName}(...args) {\n` + + 'async function reExport(...args) {\n' + ` const res = await import(${JSON.stringify(entryId)});\n` + ` return res.${currFunctionName}.call(this, ...args);\n` + - '}\n', + '}\n' + + `export { reExport as ${currFunctionName} };\n`, ), '', ); diff --git a/packages/nuxt/test/vite/utils.test.ts b/packages/nuxt/test/vite/utils.test.ts index 0d006378fc98..a38dbdc44793 100644 --- a/packages/nuxt/test/vite/utils.test.ts +++ b/packages/nuxt/test/vite/utils.test.ts @@ -88,7 +88,7 @@ describe('removeSentryQueryFromPath', () => { describe('extractFunctionReexportQueryParameters', () => { it.each([ [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}`, ['foo', 'bar']], - [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar']], + [`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar', 'default']], [ `${SENTRY_FUNCTIONS_REEXPORT}foo,a.b*c?d[e]f(g)h|i\\\\j(){hello},${QUERY_END_INDICATOR}`, ['foo', 'a\\.b\\*c\\?d\\[e\\]f\\(g\\)h\\|i\\\\\\\\j\\(\\)\\{hello\\}'], @@ -111,18 +111,36 @@ describe('constructFunctionReExport', () => { const result2 = constructFunctionReExport(query2, entryId); const expected = ` -export async function foo(...args) { +async function reExport(...args) { const res = await import("./module"); return res.foo.call(this, ...args); } -export async function bar(...args) { +export { reExport as foo }; +async function reExport(...args) { const res = await import("./module"); return res.bar.call(this, ...args); -}`; +} +export { reExport as bar }; +`; expect(result.trim()).toBe(expected.trim()); expect(result2.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 reExport(...args) { + const res = await import("./index"); + return res.default.call(this, ...args); +} +export { reExport as default }; +`; + expect(result.trim()).toBe(expected.trim()); + }); + it('returns an empty string if the query string is empty', () => { const query = ''; const entryId = './module'; From f0577d248bd18a1c4947dc0baf249a28cf266043 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 16 Oct 2024 17:11:43 +0200 Subject: [PATCH 5/5] rename option --- packages/nuxt/src/common/types.ts | 4 ++-- packages/nuxt/src/module.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/nuxt/src/common/types.ts b/packages/nuxt/src/common/types.ts index e8503d8e3dd6..6ba29752a308 100644 --- a/packages/nuxt/src/common/types.ts +++ b/packages/nuxt/src/common/types.ts @@ -110,12 +110,12 @@ export type SentryNuxtModuleOptions = { * server entry file will disable Sentry on the server-side. When you set this option to `false`, make sure * to add the Sentry server config with the node `--import` CLI flag to enable Sentry on the server-side. * - * **DO NOT** add the node CLI flag `--import` in your node start script, when `dynamicImportWrapping` is set to `true` (default). + * **DO NOT** add the node CLI flag `--import` in your node start script, when `dynamicImportForServerEntry` is set to `true` (default). * This would initialize Sentry twice on the server-side and this leads to unexpected issues. * * @default true */ - dynamicImportWrapping?: boolean; + dynamicImportForServerEntry?: boolean; /** * Options to be passed directly to the Sentry Rollup Plugin (`@sentry/rollup-plugin`) and Sentry Vite Plugin (`@sentry/vite-plugin`) that ship with the Sentry Nuxt SDK. diff --git a/packages/nuxt/src/module.ts b/packages/nuxt/src/module.ts index 579474e63fbb..77bf8edd77de 100644 --- a/packages/nuxt/src/module.ts +++ b/packages/nuxt/src/module.ts @@ -20,7 +20,7 @@ export default defineNuxtModule({ setup(moduleOptionsParam, nuxt) { const moduleOptions = { ...moduleOptionsParam, - dynamicImportWrapping: moduleOptionsParam.dynamicImportWrapping !== false, // default: true + dynamicImportForServerEntry: moduleOptionsParam.dynamicImportForServerEntry !== false, // default: true }; const moduleDirResolver = createResolver(import.meta.url); @@ -53,7 +53,7 @@ export default defineNuxtModule({ const serverConfigFile = findDefaultSdkInitFile('server'); if (serverConfigFile) { - if (moduleOptions.dynamicImportWrapping === false) { + if (moduleOptions.dynamicImportForServerEntry === false) { // Inject the server-side Sentry config file with a side effect import addPluginTemplate({ mode: 'server', @@ -63,9 +63,9 @@ export default defineNuxtModule({ 'import { defineNuxtPlugin } from "#imports"\n' + 'export default defineNuxtPlugin(() => {})', }); - - addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server')); } + + addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server')); } if (clientConfigFile || serverConfigFile) { @@ -74,7 +74,7 @@ export default defineNuxtModule({ nuxt.hooks.hook('nitro:init', nitro => { if (serverConfigFile && serverConfigFile.includes('.server.config')) { - if (moduleOptions.dynamicImportWrapping === false) { + if (moduleOptions.dynamicImportForServerEntry === false) { addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile); if (moduleOptions.debug) {