feat(nuxt): allow custom configuration files paths in Nuxt module#20650
feat(nuxt): allow custom configuration files paths in Nuxt module#20650victorgarciaesgi wants to merge 11 commits intogetsentry:developfrom
Conversation
| } | ||
|
|
||
| const serverConfigFile = findDefaultSdkInitFile('server', nuxt); | ||
| const serverConfigFile = await findDefaultSdkInitFile('server', nuxt, moduleOptions); |
There was a problem hiding this comment.
Bug: The check for serverConfigFile requires the filename to include .server.config, which silently skips server-side Sentry initialization if a custom filename is used.
Severity: HIGH
Suggested Fix
The check if (serverConfigFile?.includes('.server.config')) is too restrictive and should be removed. The presence of the serverConfigFile option should be sufficient to trigger the necessary server build steps, allowing any custom file path as intended.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/nuxt/src/module.ts#L82
Potential issue: When a custom `serverConfigFile` path is provided, a check requires the
resolved path to contain the string `.server.config`. If a user provides a custom
filename that doesn't match this convention, such as `server-sentry-init.ts`, the check
fails. This silently skips critical server build steps, including
`addServerConfigToBuild` and `addSentryTopImport`, preventing the custom configuration
from being bundled and preloaded. As a result, server-side Sentry initialization fails
without any warning, even though the feature is intended to support any file path.
Did we get this right? 👍 / 👎 to inform future reviews.
|
|
||
| expect(result).toBe(expectedPath); | ||
| expect(resolvePathMock).toHaveBeenCalledWith('~/server-sentry-config.ts'); | ||
| }); |
There was a problem hiding this comment.
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).
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit b947a37. Configure here.
There was a problem hiding this comment.
This is a Nuxt module related change, I couln't find the need for an integration test
| } | ||
| } catch { | ||
| // Fails silently as the file is optional | ||
| } |
There was a problem hiding this comment.
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)
Reviewed by Cursor Bugbot for commit 490c666. Configure here.
s1gr1d
left a comment
There was a problem hiding this comment.
Thanks for this contribution!
Just a general question: how does your setup look like? Because in case you are using Nuxt layers, a shared config already works.
| * } | ||
| * ``` | ||
| */ | ||
| clientConfigFile?: string; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@s1gr1d I implemented the single configRootDir, which is much simpler to use & implement 👍
There was a problem hiding this comment.
Sorry didn't see your comment, renamed it 👌
|
Hey @s1gr1d thanks for the fast response! We're not using layers yet, our structure basically looks like this: app-1/
|
Co-authored-by: Sigrid <32902192+s1gr1d@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 207be84. Configure here.


Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Context
I work in a big monorepo with 60+ Nuxt apps.
To enable Sentry in all those apps, they use a shared configuration, handled by an internal "proxy" module.
Each app can just install and enable this module to share the same common configuration.
The problem is: with the official Sentry module, each app would have to declare its own:
sentry.client.config.tssentry.server.config.tsWhich isn't very convenient when we have 60+ apps to enable.
Basically in the "proxy" module, I wanted to do this:
Result
This PR adds
configDiroption to the Nuxt module, allowing more flexible configuration and avoid creating asentry.**.config.tsfor each app 👌The module can now search in a custom directory path instead of the project root path.
I'm successfully running this change as a
pnpm patchon my monorepo so I wanted to contribute it.