Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nuxt): Add Rollup plugin to wrap server entry with import() #13945

Merged
merged 3 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 113 additions & 4 deletions packages/nuxt/src/vite/addServerConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@ import { createResolver } from '@nuxt/kit';
import type { Nuxt } from '@nuxt/schema';
import { consoleSandbox } from '@sentry/utils';
import type { Nitro } from 'nitropack';
import type { InputPluginOption } from 'rollup';
import type { SentryNuxtModuleOptions } from '../common/types';
import {
QUERY_END_INDICATOR,
SENTRY_FUNCTIONS_REEXPORT,
SENTRY_WRAPPED_ENTRY,
constructFunctionReExport,
removeSentryQueryFromPath,
} from './utils';

const SERVER_CONFIG_FILENAME = 'sentry.server.config';

/**
* Adds the `sentry.server.config.ts` file as `sentry.server.config.mjs` to the `.output` directory to be able to reference this file in the node --import option.
Expand All @@ -23,7 +33,7 @@ export function addServerConfigToBuild(
'server' in viteInlineConfig.build.rollupOptions.input
) {
// Create a rollup entry for the server config to add it as `sentry.server.config.mjs` to the build
(viteInlineConfig.build.rollupOptions.input as { [entryName: string]: string })['sentry.server.config'] =
(viteInlineConfig.build.rollupOptions.input as { [entryName: string]: string })[SERVER_CONFIG_FILENAME] =
createResolver(nuxt.options.srcDir).resolve(`/${serverConfigFile}`);
}

Expand All @@ -34,8 +44,8 @@ export function addServerConfigToBuild(
nitro.hooks.hook('close', async () => {
const buildDirResolver = createResolver(nitro.options.buildDir);
const serverDirResolver = createResolver(nitro.options.output.serverDir);
const source = buildDirResolver.resolve('dist/server/sentry.server.config.mjs');
const destination = serverDirResolver.resolve('sentry.server.config.mjs');
const source = buildDirResolver.resolve(`dist/server/${SERVER_CONFIG_FILENAME}.mjs`);
const destination = serverDirResolver.resolve(`${SERVER_CONFIG_FILENAME}.mjs`);

try {
await fs.promises.access(source, fs.constants.F_OK);
Expand Down Expand Up @@ -85,7 +95,7 @@ export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro

try {
fs.readFile(entryFilePath, 'utf8', (err, data) => {
const updatedContent = `import './sentry.server.config.mjs';\n${data}`;
const updatedContent = `import './${SERVER_CONFIG_FILENAME}.mjs';\n${data}`;

fs.writeFile(entryFilePath, updatedContent, 'utf8', () => {
if (moduleOptions.debug) {
Expand All @@ -111,3 +121,102 @@ export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro
}
});
}

/**
* 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.
*
* With this, the Sentry server config can be loaded before all other modules of the application (which is needed for import-in-the-middle).
* See: https://nodejs.org/api/module.html#enabling
*/
export function addDynamicImportEntryFileWrapper(nitro: Nitro, serverConfigFile: string): void {
if (!nitro.options.rollupConfig) {
nitro.options.rollupConfig = { output: {} };
}

if (nitro.options.rollupConfig?.plugins === null || nitro.options.rollupConfig?.plugins === undefined) {
nitro.options.rollupConfig.plugins = [];
} else if (!Array.isArray(nitro.options.rollupConfig.plugins)) {
// `rollupConfig.plugins` can be a single plugin, so we want to put it into an array so that we can push our own plugin
nitro.options.rollupConfig.plugins = [nitro.options.rollupConfig.plugins];
}

nitro.options.rollupConfig.plugins.push(
// @ts-expect-error - This is the correct type, but it shows an error because of two different definitions
wrapEntryWithDynamicImport(createResolver(nitro.options.srcDir).resolve(`/${serverConfigFile}`)),
);
}

/**
* A Rollup plugin which wraps the server entry with a dynamic `import()`. This makes it possible to initialize Sentry first
* by using a regular `import` and load the server after that.
* This also works with serverless `handler` functions, as it re-exports the `handler`.
*/
function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPluginOption {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: If a function is only called in one place, and the calling function is already not that big, I always consider inlining it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this function as the rollup plugin so I didn't want to inline it in the push(). However, I could add a JSDoc here for the plugin.

return {
name: 'sentry-wrap-entry-with-dynamic-import',
async resolveId(source, importer, options) {
if (source.includes(`/${SERVER_CONFIG_FILENAME}`)) {
return { id: source, moduleSideEffects: true };
}

if (source === 'import-in-the-middle/hook.mjs') {
// We are importing "import-in-the-middle" in the returned code of the `load()` function below
// By setting `moduleSideEffects` to `true`, the import is added to the bundle, although nothing is imported from it
// By importing "import-in-the-middle/hook.mjs", we can make sure this file is included, as not all node builders are including files imported with `module.register()`.
// Prevents the error "Failed to register ESM hook Error: Cannot find module 'import-in-the-middle/hook.mjs'"
return { id: source, moduleSideEffects: true, external: true };
}

if (options.isEntry && !source.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)) {
const resolution = await this.resolve(source, importer, options);

// If it cannot be resolved or is external, just return it so that Rollup can display an error
if (!resolution || resolution?.external) return resolution;

const moduleInfo = await this.load(resolution);

moduleInfo.moduleSideEffects = true;

// The key `.` in `exportedBindings` refer to the exports within the file
const exportedFunctions = moduleInfo.exportedBindings?.['.'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Quick comment what the '.' is about?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the rollup docs it's the "path of from". As the . entry includes the exports from the file I figured the dot means exactly that: The exports from this file 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I know but I think we should add a comment about that 😄


// 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(
exportedFunctions?.length
? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')).concat(QUERY_END_INDICATOR)
: '',
);
}
return null;
},
load(id: string) {
if (id.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)) {
const entryId = removeSentryQueryFromPath(id);

// Mostly useful for serverless `handler` functions
const reExportedFunctions = id.includes(SENTRY_FUNCTIONS_REEXPORT)
? constructFunctionReExport(id, entryId)
: '';

return (
// Regular `import` of the Sentry config
`import ${JSON.stringify(resolvedSentryConfigPath)};\n` +
// Dynamic `import()` for the previous, actual entry point.
// `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" +
`${reExportedFunctions}\n`
);
}

return null;
},
};
}
56 changes: 56 additions & 0 deletions packages/nuxt/src/vite/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,59 @@ export function findDefaultSdkInitFile(type: 'server' | 'client'): string | unde

return filePaths.find(filename => fs.existsSync(filename));
}

export const SENTRY_WRAPPED_ENTRY = '?sentry-query-wrapped-entry';
export const SENTRY_FUNCTIONS_REEXPORT = '?sentry-query-functions-reexport=';
export const QUERY_END_INDICATOR = 'SENTRY-QUERY-END';

/**
* Strips the Sentry query part from a path.
* Example: example/path?sentry-query-wrapped-entry?sentry-query-functions-reexport=foo,SENTRY-QUERY-END -> /example/path
*
* Only exported for testing.
*/
export function removeSentryQueryFromPath(url: string): string {
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
const regex = new RegExp(`\\${SENTRY_WRAPPED_ENTRY}.*?\\${QUERY_END_INDICATOR}`);
return url.replace(regex, '');
}

/**
* Extracts and sanitizes function re-export query parameters from a query string.
* If it is a default export, it is not considered for re-exporting. This function is mostly relevant for re-exporting
* serverless `handler` functions.
*
* Only exported for testing.
*/
export function extractFunctionReexportQueryParameters(query: string): string[] {
// Regex matches the comma-separated params between the functions query
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
const regex = new RegExp(`\\${SENTRY_FUNCTIONS_REEXPORT}(.*?)\\${QUERY_END_INDICATOR}`);
const match = query.match(regex);

return match && match[1]
? match[1]
.split(',')
.filter(param => param !== '' && param !== 'default')
// Sanitize, as code could be injected with another rollup plugin
.map((str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What cases are we sanitizing for here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function gets the functions from a file to re-export them. Theoretically, another rollup plugin could inject code here. As discussed in person, this is code a user can control so it is unlikely to be used by attackers but I'll keep it with a small comment - just to be safe.

: [];
}

/**
* 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) =>
functionsCode.concat(
`export async function ${currFunctionName}(...args) {\n` +
` const res = await import(${JSON.stringify(entryId)});\n` +
` return res.${currFunctionName}.call(this, ...args);\n` +
'}\n',
),
'',
);
}
70 changes: 69 additions & 1 deletion packages/nuxt/test/vite/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import * as fs from 'fs';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { findDefaultSdkInitFile } from '../../src/vite/utils';
import {
QUERY_END_INDICATOR,
SENTRY_FUNCTIONS_REEXPORT,
SENTRY_WRAPPED_ENTRY,
constructFunctionReExport,
extractFunctionReexportQueryParameters,
findDefaultSdkInitFile,
removeSentryQueryFromPath,
} from '../../src/vite/utils';

vi.mock('fs');

Expand Down Expand Up @@ -59,3 +67,63 @@ describe('findDefaultSdkInitFile', () => {
expect(result).toMatch('packages/nuxt/sentry.server.config.js');
});
});

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 result = removeSentryQueryFromPath(url);
expect(result).toBe('/example/path');
});

it('returns the same path if the specific query part is not present', () => {
const url = '/example/path?other-query=param';
const result = removeSentryQueryFromPath(url);
expect(result).toBe(url);
});
});

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,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\\}'],
],
[`/example/path/${SENTRY_FUNCTIONS_REEXPORT}foo,bar${QUERY_END_INDICATOR}`, ['foo', 'bar']],
[`${SENTRY_FUNCTIONS_REEXPORT}${QUERY_END_INDICATOR}`, []],
['?other-query=param', []],
])('extracts parameters from the query string: %s', (query, expected) => {
const result = extractFunctionReexportQueryParameters(query);
expect(result).toEqual(expected);
});
});

describe('constructFunctionReExport', () => {
it('constructs re-export code for given query parameters and entry ID', () => {
const query = `${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}}`;
const query2 = `${SENTRY_FUNCTIONS_REEXPORT}foo,bar${QUERY_END_INDICATOR}}`;
const entryId = './module';
const result = constructFunctionReExport(query, entryId);
const result2 = constructFunctionReExport(query2, entryId);

const expected = `
export async function foo(...args) {
const res = await import("./module");
return res.foo.call(this, ...args);
}
export async function bar(...args) {
const res = await import("./module");
return res.bar.call(this, ...args);
}`;
expect(result.trim()).toBe(expected.trim());
expect(result2.trim()).toBe(expected.trim());
});

it('returns an empty string if the query string is empty', () => {
const query = '';
const entryId = './module';
const result = constructFunctionReExport(query, entryId);
expect(result).toBe('');
});
});
Loading