feat(nitro): Handle sourcemap preparation and upload#19304
feat(nitro): Handle sourcemap preparation and upload#19304logaretm wants to merge 6 commits intoawad/js-1128-nitro-use-tracing-channels-for-srvx-and-h3from
Conversation
Codecov Results 📊Generated by Codecov Action |
size-limit report 📦
|
logaretm
force-pushed
the
awad/js-1128-nitro-use-tracing-channels-for-srvx-and-h3
branch
from
1923dda to
be2f037
Compare
logaretm
force-pushed
the
awad/js-1076-nitro-upload-sourcemaps
branch
from
0d86a79 to
9f6254a
Compare
logaretm
force-pushed
the
awad/js-1128-nitro-use-tracing-channels-for-srvx-and-h3
branch
from
be2f037 to
3b30e36
Compare
logaretm
force-pushed
the
awad/js-1076-nitro-upload-sourcemaps
branch
from
9f6254a to
63a2175
Compare
logaretm
force-pushed
the
awad/js-1128-nitro-use-tracing-channels-for-srvx-and-h3
branch
from
3b30e36 to
36c96b6
Compare
logaretm
force-pushed
the
awad/js-1076-nitro-upload-sourcemaps
branch
2 times, most recently
from
d3e3ae1 to
a24cad5
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you apply the label |
logaretm
force-pushed
the
awad/js-1128-nitro-use-tracing-channels-for-srvx-and-h3
branch
2 times, most recently
from
3cf1515 to
342ea3b
Compare
logaretm
force-pushed
the
awad/js-1076-nitro-upload-sourcemaps
branch
from
a24cad5 to
1583866
Compare
logaretm
force-pushed
the
awad/js-1128-nitro-use-tracing-channels-for-srvx-and-h3
branch
from
342ea3b to
8d44437
Compare
logaretm
force-pushed
the
awad/js-1076-nitro-upload-sourcemaps
branch
from
1583866 to
07659b6
Compare
logaretm
force-pushed
the
awad/js-1076-nitro-upload-sourcemaps
branch
from
a967e7a to
00bd835
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (options?.sourcemaps?.disable !== 'disable-upload') { | ||
| await sentryBuildPluginManager.injectDebugIds([outputDir]); | ||
| await sentryBuildPluginManager.uploadSourcemaps([outputDir], { | ||
| // We don't prepare the artifacts because we injected debug IDs manually before | ||
| prepareArtifacts: false, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: When sourcemaps.disable is set to 'disable-upload', the code incorrectly skips injecting debug IDs, not just the source map upload, breaking manual upload workflows.
Severity: HIGH
Suggested Fix
Move the injectDebugIds() call out of the conditional block that checks options?.sourcemaps?.disable !== 'disable-upload'. This check should only wrap the uploadSourcemaps() call, ensuring debug IDs are injected even when automatic uploads are disabled.
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/nitro/src/sourceMaps.ts#L42-L48
Potential issue: The logic in `handleSourceMapUpload` incorrectly skips debug ID
injection when source map uploads are disabled. The condition `if
(options?.sourcemaps?.disable !== 'disable-upload')` wraps both the `injectDebugIds()`
and `uploadSourcemaps()` calls. According to documentation, when
`options.sourcemaps.disable` is set to `'disable-upload'`, the plugin should still
inject debug IDs to support manual uploads, but skip the automatic upload step. The
current implementation skips both actions, which means build artifacts will lack the
necessary debug IDs for users relying on a manual upload workflow.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Pull request overview
Adds build-time sourcemap generation + post-build sourcemap (and debug ID) handling to the @sentry/nitro SDK by wiring Nitro’s compiled hook to @sentry/bundler-plugin-core.
Changes:
- Introduces
packages/nitro/src/sourceMaps.tsto configure Nitro sourcemap settings and to run a post-build upload flow viacreateSentryBuildPluginManager. - Threads
SentryNitroOptionsthrough module setup so sourcemap behavior can be configured fromwithSentryConfig/setupSentryNitroModule. - Adds Vitest coverage for option-shaping and basic hook registration behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/nitro/src/sourceMaps.ts | New sourcemap config + post-build hook which injects debug IDs and uploads sourcemaps via bundler-plugin-core |
| packages/nitro/test/sourceMaps.test.ts | New unit tests for plugin option building, config mutation, and hook registration |
| packages/nitro/src/module.ts | Passes Sentry options into module setup to enable sourcemap handling |
| packages/nitro/src/config.ts | Defines SentryNitroOptions from bundler-plugin-core options and enables sourcemap config during module setup |
| packages/nitro/rollup.npm.config.mjs | Marks @sentry/bundler-plugin-core as external in the package build |
| packages/nitro/package.json | Adds @sentry/bundler-plugin-core dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (options?.sourcemaps?.disable !== 'disable-upload') { | ||
| await sentryBuildPluginManager.injectDebugIds([outputDir]); | ||
| await sentryBuildPluginManager.uploadSourcemaps([outputDir], { | ||
| // We don't prepare the artifacts because we injected debug IDs manually before | ||
| prepareArtifacts: false, | ||
| }); | ||
| } |
| } | ||
|
|
||
| await sentryBuildPluginManager.deleteArtifacts(); |
| telemetry: { | ||
| metaFramework: 'nitro', | ||
| }, | ||
| ...options?._metaOptions, |
| import type { Options } from '@sentry/bundler-plugin-core'; | ||
| import { createSentryBuildPluginManager } from '@sentry/bundler-plugin-core'; | ||
| import { debug } from '@sentry/core'; | ||
| import type { Nitro, NitroConfig } from 'nitro/types'; | ||
| import type { SentryNitroOptions } from './config'; |
| // This makes sourcemaps unusable for Sentry. | ||
| // FIXME: Not sure about this one, it works either way? |
| describe('setupSourceMaps', () => { | ||
| it('does not register hook in dev mode', () => { | ||
| const hookFn = vi.fn(); | ||
| const nitro = { | ||
| options: { dev: true, output: { serverDir: '/output/server' } }, | ||
| hooks: { hook: hookFn }, | ||
| } as any; | ||
|
|
||
| setupSourceMaps(nitro); | ||
|
|
||
| expect(hookFn).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does not register hook when sourcemaps.disable is true', () => { | ||
| const hookFn = vi.fn(); | ||
| const nitro = { | ||
| options: { dev: false, output: { serverDir: '/output/server' } }, | ||
| hooks: { hook: hookFn }, | ||
| } as any; | ||
|
|
||
| setupSourceMaps(nitro, { sourcemaps: { disable: true } }); | ||
|
|
||
| expect(hookFn).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does not register hook when disable is true', () => { | ||
| const hookFn = vi.fn(); | ||
| const nitro = { | ||
| options: { dev: false, output: { serverDir: '/output/server' } }, | ||
| hooks: { hook: hookFn }, | ||
| } as any; | ||
|
|
||
| setupSourceMaps(nitro, { disable: true }); | ||
|
|
||
| expect(hookFn).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('registers compiled hook in production mode', () => { | ||
| const hookFn = vi.fn(); | ||
| const nitro = { | ||
| options: { dev: false, output: { serverDir: '/output/server' } }, | ||
| hooks: { hook: hookFn }, | ||
| } as any; | ||
|
|
||
| setupSourceMaps(nitro); | ||
|
|
||
| expect(hookFn).toHaveBeenCalledWith('compiled', expect.any(Function)); | ||
| }); | ||
|
|
||
| it('registers compiled hook with custom options', () => { | ||
| const hookFn = vi.fn(); | ||
| const nitro = { | ||
| options: { dev: false, output: { serverDir: '/output/server' } }, | ||
| hooks: { hook: hookFn }, | ||
| } as any; | ||
|
|
||
| setupSourceMaps(nitro, { org: 'my-org', project: 'my-project' }); | ||
|
|
||
| expect(hookFn).toHaveBeenCalledWith('compiled', expect.any(Function)); | ||
| }); |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 973bf37. Configure here.
| // We don't prepare the artifacts because we injected debug IDs manually before | ||
| prepareArtifacts: false, | ||
| }); | ||
| } |
There was a problem hiding this comment.
disable-upload incorrectly skips debug ID injection
High Severity
When sourcemaps.disable is 'disable-upload', the condition options?.sourcemaps?.disable !== 'disable-upload' causes both injectDebugIds and uploadSourcemaps to be skipped. According to the @sentry/bundler-plugin-core documentation, disable-upload means "will not upload source maps but will inject debug IDs." The injectDebugIds call needs to happen outside this conditional so it still runs in disable-upload mode. The Next.js SDK handles this correctly by separating the two operations.
Reviewed by Cursor Bugbot for commit 973bf37. Configure here.
| metaFramework: 'nitro', | ||
| }, | ||
| ...options?._metaOptions, | ||
| }, |
There was a problem hiding this comment.
_metaOptions spread can override metaFramework: 'nitro'
Low Severity
The spread ...options?._metaOptions comes after the telemetry property, so any user-provided _metaOptions with a telemetry key will completely overwrite metaFramework: 'nitro'. The corresponding test asserts this value is "always" set to 'nitro', but the implementation doesn't guarantee it. The spread and default should be ordered to preserve the metaFramework value.
Reviewed by Cursor Bugbot for commit 973bf37. Configure here.
| assets: options?.sourcemaps?.assets, | ||
| ignore: options?.sourcemaps?.ignore, | ||
| filesToDeleteAfterUpload: options?.sourcemaps?.filesToDeleteAfterUpload ?? ['**/*.map'], | ||
| rewriteSources: (source: string) => normalizePath(source), |
There was a problem hiding this comment.
User-provided rewriteSources option silently ignored
Medium Severity
The rewriteSources property in getPluginOptions is hardcoded to normalizePath and ignores any user-provided options?.sourcemaps?.rewriteSources. Since SentryNitroOptions picks the full sourcemaps type (which includes rewriteSources), users can set this option but it's silently dropped. The Nuxt SDK handles this correctly with sourcemapsOptions.rewriteSources ?? normalizePath, falling back to the default only when the user doesn't provide one.
Reviewed by Cursor Bugbot for commit 973bf37. Configure here.
Adds automatic sourcemap handling to the Nitro SDK, using
@sentry/bundler-plugin-corefor builder-agnostic post-build upload.Nitro uses rollup or rolldown, so it made sense to make it as agnostic as possible.
Closes #17992
This PR is part of a stack: