Repository navigation
fix(sentry): give the build the org and project it needs to create a release (#16) - #20
Conversation
…release @Sentry/vite-plugin only creates a release and uploads source maps when org, project, authToken and a release name are all present at build time. It reports a missing one with logger.warn and returns — after it has already injected the debug IDs during the bundle phase. So a build with nothing configured and a build that worked perfectly produce indistinguishable artifacts: both have debug IDs in the bundle, and both build green. SENTRY_ORG and SENTRY_PROJECT were Fly *runtime* secrets, which the Docker build cannot see. The plugin's org guard therefore fired on every deploy since 2024-04-16: no release was ever created, and no source map was ever uploaded. Worse, filesToDeleteAfterUpload is honoured in the finally of writeBundle rather than only after a successful upload, so the maps were deleted anyway — leaving symbolication broken on the client too, with nothing left to debug it with. See #16. Passes both as --build-secret, mounts and exports them in the build stage alongside the token, and turns the remaining silent skips into a build failure: the plugin has exactly one chance to upload, so a red build is far cheaper than a release that silently does not exist. tests/sentry-release.test.ts links the three files that have to agree — the workflow that supplies the values, the Dockerfile that mounts them, and the config that reads them. Nothing in the type system did, so dropping one flag was a small edit that disabled the whole pipeline without failing a check.
…t text
Interpolating ${{ secrets.* }} puts the value into the rendered command.
The step already passes FLY_API_TOKEN through `env:`, so this follows the
file's own convention and keeps the secrets out of the script body.
Adds a test for the resulting indirection: a --build-secret whose shell
variable has no matching `env:` entry would forward the literal string
$SENTRY_ORG, and the build would fail far from the cause.
The first pass made the *wiring* correct, but a review of the installed plugin turned up five ways a build could still end up green with no release, no source maps, and no evidence either was attempted. - errorHandler: the plugin's documented default is to throw and stop the build; the shipped 11.0.0 implementation logs and continues. Restoring the documented behaviour is what makes the guard in this file mean anything, because `filesToDeleteAfterUpload` is honoured in the `finally` of its writeBundle hook whether or not the upload succeeded. A swallowed failure deleted every source map from the image and still exited zero. - setCommits: dropped. Naming `auto` explicitly opts out of the `shouldNotThrowOnFailure` the plugin's own default carries, so a failure rethrew and skipped `finalizeRelease`, leaving the release stuck in `new` on every deploy. It could not have succeeded anyway — the base image has no `git`, so there is no repository for `--auto` to read. - required=true on the secret mounts. The `&&` chain does not do this: `export X="$(cat /run/secrets/X)"` exits 0 even when the cat fails, so a missing secret arrived as an empty string rather than stopping the build. - The deploy step now rejects an empty secret before calling flyctl. An unset GitHub secret is the empty string, not an unset variable, so a deleted SENTRY_AUTH_TOKEN took the "release management is off" branch: green build, no release, no debug IDs, nothing in the log. - .env.sentry-build-plugin is now gitignored and dockerignored. The plugin merges it into process.env, and it holds an auth token; neither ignore file covered it, and `ADD . .` copies it into both image stages. Also treats an empty value as absent rather than merely undefined, tests getSentryPlugin() on both branches, and guards the build/runtime boundary: the three build-only credentials are asserted to be unread by the running app, which is what makes removing them from the Fly runtime environment safe. That removal is the outstanding operational step of #16.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c9a9a756d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const missing = Object.entries(env) | ||
| .filter(([, value]) => !value) | ||
| .map(([name]) => name) | ||
| if (missing.length > 0) { |
There was a problem hiding this comment.
When SENTRY_AUTH_TOKEN and COMMIT_SHA are exported but SENTRY_ORG or SENTRY_PROJECT are stored in the supported .env.sentry-build-plugin file, this check throws before sentryVitePlugin() is constructed; construction is what loads that file into process.env. Production-like local or CI builds using the plugin's environment file therefore fail despite supplying valid credentials. Load the file before validating, or validate the plugin-normalized environment instead.
Useful? React with 👍 / 👎.
| const loadUnder = async (env: Record<string, string | undefined>) => { | ||
| vi.resetModules() | ||
| vi.unstubAllEnvs() | ||
| for (const [name, value] of Object.entries(env)) { | ||
| vi.stubEnv(name, value ?? '') | ||
| } |
There was a problem hiding this comment.
When a developer runs the suite with any Sentry credentials exported in the shell, vi.unstubAllEnvs() restores those real values and this helper only replaces names present in env. For example, the no-token case supplies only NODE_ENV, so an inherited SENTRY_AUTH_TOKEN can make the config import reject for missing values or make getSentryPlugin() non-null, causing an environment-dependent failure. Stub all four Sentry build inputs to empty before applying each case's overrides.
Useful? React with 👍 / 👎.
1 participant
Closes #16.
@sentry/vite-pluginonly creates a Sentry release and uploads source maps whenorg,project,authTokenand a release name are all present at build time. It reports a missing one withlogger.warnand returns — after it has already injected the debug IDs during the bundle phase. So a build with nothing configured and a build that worked perfectly produce indistinguishable artifacts: both have debug IDs in the bundle, and both build green.SENTRY_ORGandSENTRY_PROJECTwere Fly runtime secrets, which the Docker build cannot see. The plugin'sorgguard therefore fired on every deploy since 2024-04-16: no release was ever created, and no source map was ever uploaded.Verified on the live app before the change:
Zero source maps, client and server. That is the part the original issue understates:
filesToDeleteAfterUploadis honoured in thefinallyof the plugin'swriteBundlehook whether or not the upload succeeded, so the maps were deleted anyway. Symbolication has been broken on both halves, with nothing left to debug it with.What changed
deploy.ymlSENTRY_ORG/SENTRY_PROJECTas--build-secret, from a stepenv:rather than${{ }}interpolation; rejects an empty secret before callingflyctlother/Dockerfilerequired=truevite.config.tssetCommitsthat skippedfinalizeRelease.gitignore,other/Dockerfile.dockerignore.env.sentry-build-plugin(holds an auth token; the plugin merges it intoprocess.env)tests/sentry-release.test.tsFive ways a release could still silently not happen, and what closes each
11.0.0logs and continues. Measured on this repo against a stub returning 500 for everything:EXIT=0, twoCouldn't finish all operationserrors, andfind build -name '*.map'went from 40 files to 0.errorHandler: (error) => { throw error }restores the contract.setCommits: { auto: true }could never work, and made it worse. The base image has nogit(node:22.23.3-bookworm-slim→git: not found), so--autohas no repository to read. Worse, namingautoexplicitly opts out of theshouldNotThrowOnFailure: truethe plugin's own default carries, so the failure rethrew and skippedfinalizeRelease— a release created and left innewforever, on every deploy. Omitting the option restores the resilient default. Commit association still does not happen; it needs a Sentry GitHub integration or asentry-cli releases set-commitsrun from the deploy job, which has a checkout. Not in scope here, noted so it is not mistaken for working.&&chain did not fail the build.export X="$(cat /run/secrets/X)" && echoexits 0 even when thecatfails — a failed command substitution does not propagate throughexport. A missing secret became an empty string.required=trueis what actually enforces it."", not unset. So a deletedSENTRY_AUTH_TOKENtook the "release management is off" branch: green deploy, no release, no debug IDs, nothing in the log. The deploy step now checks first..env.sentry-build-pluginwas in neither ignore file. The plugin merges it intoprocess.envat construction, it holds an auth token, andADD . .copies it into both image stages. It can also setSENTRY_URL, which nothing in the config passes, redirecting the release and upload to another host with the token attached.Why a test, when the issue said none was practical
The issue's position was that the wiring only manifests inside a remote Docker build. True of the values —
getSentryPlugin()now refuses to build when one is absent, and a test here could not add to that, because by then the guard and the plugin are reading the same empty variable and agreeing with each other.But the issue also said the half worth guarding was the removal, "because it is the part that can silently break production." That half is mechanically testable:
tests/sentry-release.test.tsasserts the three build-only credentials are never read byapp/,server/orindex.ts, and thatSENTRY_DSNstill is. Verified non-vacuous — adding oneprocess.env.SENTRY_ORGtoserver/index.tsfails the test, and the scan covers 76 files.The wiring itself is tested by set equality, not a hardcoded list: the names come from matching
process.env.…invite.config.ts, so adding one togetSentryEnv()demands the workflow flag, the mount and the export for it. Verified by mutation — removing the--build-secretline fails 2 tests, the--mount1, theexport1, the stepenv:entry 1.Not in this PR
flyctl secrets unsetof the three build-only secrets from the Fly runtime environment. Deliberately not done here: it destroys the only record of the org/project slugs and restarts the app, so it runs immediately after this deploys and is verified green. The values are already in GitHub Actions secrets, captured from the live container before the unset.releaseyet.COMMIT_SHAstill does not cross into the runtime image, so server events remain unattributed after this merges. That is Server-side Sentry events carry no release — COMMIT_SHA is dropped at the end of the Docker build stage #15, and it has to land for server symbolication to work — this PR makes the release exist and the maps upload, Server-side Sentry events carry no release — COMMIT_SHA is dropped at the end of the Docker build stage #15 makes the server tag events with it.SENTRY_AUTH_TOKENrotation. It is a legacy 64-hex token rather than an org-scopedsntrys_one, and it has been readable in the production process environment since 2024-04-16. Ansntrys_token would also remove the need forSENTRY_ORGentirely. Separate change, deliberately not folded in here.Verified
npm run validateclean ·npm run test:run→ 14 files, 199 tests ·npm run buildok.Note for whoever verifies by hand: a release appearing in Sentry under the full SHA is not sufficient evidence, and neither is the absence of a warning. The original evidence in #15 was
_sentryDebugIdsin the bundle, which proves only that the plugin ran. Check that the release is finalized and has associated commits; and read the build log forSuccessfully uploaded source maps, not for the absence of an error.