Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
32 changes: 32 additions & 0 deletions packages/nuxt/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,38 @@ export type SentryNuxtModuleOptions = BuildTimeOptionsBase & {
*/
autoInjectServerSentry?: 'top-level-import' | 'experimental_dynamic-import';

/**
* Provide the resolved path to a custom Sentry client config file.
*
* If not provided, the default location (`<projectRoot>/sentry.client.config.(js|ts)`) will be used to look up the config file.
* If there is no file at the default location either, the SDK will initialize with the options specified in the `sentry` module options or with default options.
Comment thread
victorgarciaesgi marked this conversation as resolved.
Outdated
*
* @example
*
* ```ts
* sentry: {
* clientConfigFile: '~/client-sentry-config.ts',
* }
* ```
*/
clientConfigFile?: string;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What do you think about adding clientConfigPath and serverConfigPath?
So you can provide a specific pathname to a folder where the SDK will look for the config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep that seems better i'll change it 👍
I've not handled the case of a folder path, could a single configRootDir: string work better? That way it works for both client and server

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

True, that would be even better. You could call it configDir. The root part is a bit misleading as it's just a simple path for where we can find the config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@s1gr1d I implemented the single configRootDir, which is much simpler to use & implement 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry didn't see your comment, renamed it 👌


/**
* Provide the resolved path to a custom Sentry server config file.
*
* If not provided, the default location (`<projectRoot>/sentry.server.config.(js|ts)`) will be used to look up the config file.
* If there is no file at the default location either, the SDK will initialize with the options specified in the `sentry` module options or with default options.
*
* @example
*
* ```ts
* sentry: {
* serverConfigFile: '~/server-sentry-config.ts',
* }
* ```
*/
serverConfigFile?: string;

/**
* When `autoInjectServerSentry` is set to `"experimental_dynamic-import"`, the SDK will wrap your Nitro server entrypoint
* with a dynamic `import()` to ensure all dependencies can be properly instrumented. Any previous exports from the entrypoint are still exported.
Expand Down
4 changes: 2 additions & 2 deletions packages/nuxt/src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default defineNuxtModule<ModuleOptions>({
const moduleDirResolver = createResolver(import.meta.url);
const buildDirResolver = createResolver(nuxt.options.buildDir);

const clientConfigFile = findDefaultSdkInitFile('client', nuxt);
const clientConfigFile = await findDefaultSdkInitFile('client', nuxt, moduleOptions);

if (clientConfigFile) {
// Inject the client-side Sentry config file with a side effect import
Expand Down Expand Up @@ -78,7 +78,7 @@ export default defineNuxtModule<ModuleOptions>({
});
}

const serverConfigFile = findDefaultSdkInitFile('server', nuxt);
const serverConfigFile = await findDefaultSdkInitFile('server', nuxt, moduleOptions);
Comment thread
victorgarciaesgi marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
const isNitroV3 = (await getNitroMajorVersion()) >= 3;
const nuxtMajor = parseInt((nuxt as unknown as { _version: string })._version?.split('.')[0] ?? '3', 10);
const isMinNuxtV4 = nuxtMajor >= 4;
Expand Down
30 changes: 29 additions & 1 deletion packages/nuxt/src/vite/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import type { Nuxt } from '@nuxt/schema';
import { consoleSandbox } from '@sentry/core';
import * as fs from 'fs';
import * as path from 'path';
import type { SentryNuxtModuleOptions } from '../common/types';
import { resolvePath } from '@nuxt/kit';

/**
* Gets the major version of the installed nitro package.
Expand All @@ -25,16 +27,42 @@ export async function getNitroMajorVersion(): Promise<number> {
* Find the default SDK init file for the given type (client or server).
* The sentry.server.config file is prioritized over the instrument.server file.
*/
export function findDefaultSdkInitFile(type: 'server' | 'client', nuxt?: Nuxt): string | undefined {
export async function findDefaultSdkInitFile(
type: 'server' | 'client',
nuxt?: Nuxt,
options?: SentryNuxtModuleOptions,
): Promise<string | undefined> {
const possibleFileExtensions = ['ts', 'js', 'mjs', 'cjs', 'mts', 'cts'];
const relativePaths: string[] = [];

if (type === 'server') {
if (options?.serverConfigFile) {
try {
const resolvedPath = await resolvePath(options.serverConfigFile);
if (fs.existsSync(resolvedPath)) {
return resolvedPath;
}
throw new Error(`Server configuration file not found: ${resolvedPath}`);
} catch (e) {
throw new Error(`Error resolving server configuration file: ${options.serverConfigFile}. Cause: ${e}`);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Custom server config paths skip critical server-side setup

High Severity

When a custom serverConfigFile path is provided that doesn't contain the substring .server.config (e.g. the documented example ~/server-sentry-config.ts), the guard at module.ts line 173 (serverConfigFile?.includes('.server.config')) evaluates to false. This silently skips all critical server-side setup: addServerConfigToBuild, addSentryTopImport, and addDynamicImportEntryFileWrapper. The server SDK will register plugins but never actually load the config file, making autoInjectServerSentry non-functional for custom-named config files.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 490c666. Configure here.

Comment thread
sentry[bot] marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
}
for (const ext of possibleFileExtensions) {
relativePaths.push(`sentry.${type}.config.${ext}`);
relativePaths.push(path.join('public', `instrument.${type}.${ext}`));
}
} else {
if (options?.clientConfigFile) {
try {
const resolvedPath = await resolvePath(options.clientConfigFile);
if (fs.existsSync(resolvedPath)) {
return resolvedPath;
}
throw new Error(`Client configuration file not found: ${resolvedPath}`);
} catch (e) {
throw new Error(`Error resolving client configuration file: ${options.clientConfigFile}. Cause: ${e}`);
}
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
}
for (const ext of possibleFileExtensions) {
relativePaths.push(`sentry.${type}.config.${ext}`);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/nuxt/test/vite/buildOptions.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ describe('Sentry Nuxt build-time options type', () => {
// --- SentryNuxtModuleOptions specific options ---
enabled: true,
autoInjectServerSentry: 'experimental_dynamic-import',
clientConfigFile: '~/client-sentry-config.ts',
serverConfigFile: '~/server-sentry-config.ts',
experimental_entrypointWrappedFunctions: ['default', 'handler', 'server', 'customExport'],
unstable_sentryBundlerPluginOptions: {
// Rollup plugin options
Expand Down
72 changes: 56 additions & 16 deletions packages/nuxt/test/vite/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ import {
SENTRY_WRAPPED_FUNCTIONS,
} from '../../src/vite/utils';


const resolvePathMock = vi.hoisted(() => vi.fn());

vi.mock('@nuxt/kit', () => ({
resolvePath: resolvePathMock,
}));

vi.mock('fs');

describe('findDefaultSdkInitFile', () => {
Expand All @@ -24,43 +31,76 @@ describe('findDefaultSdkInitFile', () => {

it.each(['ts', 'js', 'mjs', 'cjs', 'mts', 'cts'])(
'should return the server file path with .%s extension if it exists',
ext => {
async ext => {
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return !(filePath instanceof URL) && filePath.toString().includes(`sentry.server.config.${ext}`);
});

const result = findDefaultSdkInitFile('server');
const result = await findDefaultSdkInitFile('server');
expect(result).toMatch(`packages/nuxt/sentry.server.config.${ext}`);
},
);

it.each(['ts', 'js', 'mjs', 'cjs', 'mts', 'cts'])(
'should return the client file path with .%s extension if it exists',
ext => {
async ext => {
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return !(filePath instanceof URL) && filePath.toString().includes(`sentry.client.config.${ext}`);
});

const result = findDefaultSdkInitFile('client');
const result = await findDefaultSdkInitFile('client');
expect(result).toMatch(`packages/nuxt/sentry.client.config.${ext}`);
},
);

it('should return undefined if no file with specified extensions exists', () => {
it('should return a custom client config file if it exists', async () => {
const expectedPath = '/some/path/mock-app/app/client-sentry-config.ts';

vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return !(filePath instanceof URL) && filePath.toString() === expectedPath;
});

resolvePathMock.mockResolvedValue(expectedPath);

const result = await findDefaultSdkInitFile('client', undefined, {
clientConfigFile: '~/client-sentry-config.ts',
});

expect(result).toBe(expectedPath);
expect(resolvePathMock).toHaveBeenCalledWith('~/client-sentry-config.ts');
});

it('should return a custom server config file if it exists', async () => {
const expectedPath = '/users/some-user/front-example/app/server-sentry-config.ts';
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return !(filePath instanceof URL) && filePath.toString() === expectedPath;
});

resolvePathMock.mockResolvedValue(expectedPath);

const result = await findDefaultSdkInitFile('server', undefined, {
serverConfigFile: '~/server-sentry-config.ts',
});

expect(result).toBe(expectedPath);
expect(resolvePathMock).toHaveBeenCalledWith('~/server-sentry-config.ts');
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Feat PR lacks integration or E2E test coverage

Low Severity

This feat PR only includes unit tests for findDefaultSdkInitFile and a type test. Per project rules, feat PRs need at least one integration or E2E test to verify the full feature works end-to-end (e.g., that a custom config file is correctly picked up and wired through the Nuxt module setup).

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit b947a37. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a Nuxt module related change, I couln't find the need for an integration test


it('should return undefined if no file with specified extensions exists', async () => {
vi.spyOn(fs, 'existsSync').mockReturnValue(false);

const result = findDefaultSdkInitFile('server');
const result = await findDefaultSdkInitFile('server');
expect(result).toBeUndefined();
});

it('should return undefined if no file exists', () => {
it('should return undefined if no file exists', async () => {
vi.spyOn(fs, 'existsSync').mockReturnValue(false);

const result = findDefaultSdkInitFile('server');
const result = await findDefaultSdkInitFile('server');
expect(result).toBeUndefined();
});

it('should return the server config file path if server.config and instrument exist', () => {
it('should return the server config file path if server.config and instrument exist', async () => {
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return (
!(filePath instanceof URL) &&
Expand All @@ -69,11 +109,11 @@ describe('findDefaultSdkInitFile', () => {
);
});

const result = findDefaultSdkInitFile('server');
const result = await findDefaultSdkInitFile('server');
expect(result).toMatch('packages/nuxt/sentry.server.config.js');
});

it('should return the latest layer config file path if client config exists', () => {
it('should return the latest layer config file path if client config exists', async () => {
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return !(filePath instanceof URL) && filePath.toString().includes('sentry.client.config.ts');
});
Expand All @@ -91,11 +131,11 @@ describe('findDefaultSdkInitFile', () => {
},
} as unknown as Nuxt;

const result = findDefaultSdkInitFile('client', nuxtMock);
const result = await findDefaultSdkInitFile('client', nuxtMock);
expect(result).toMatch('packages/nuxt/sentry.client.config.ts');
});

it('should return the latest layer config file path if server config exists', () => {
it('should return the latest layer config file path if server config exists', async () => {
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return (
!(filePath instanceof URL) &&
Expand All @@ -117,11 +157,11 @@ describe('findDefaultSdkInitFile', () => {
},
} as unknown as Nuxt;

const result = findDefaultSdkInitFile('server', nuxtMock);
const result = await findDefaultSdkInitFile('server', nuxtMock);
expect(result).toMatch('packages/nuxt/sentry.server.config.ts');
});

it('should return the latest layer config file path if client config exists in former layer', () => {
it('should return the latest layer config file path if client config exists in former layer', async () => {
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => {
return !(filePath instanceof URL) && filePath.toString().includes('nuxt/sentry.client.config.ts');
});
Expand All @@ -139,7 +179,7 @@ describe('findDefaultSdkInitFile', () => {
},
} as unknown as Nuxt;

const result = findDefaultSdkInitFile('client', nuxtMock);
const result = await findDefaultSdkInitFile('client', nuxtMock);
expect(result).toMatch('packages/nuxt/sentry.client.config.ts');
});
});
Expand Down