-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(v9/remix): Ensure source maps upload fails silently if Sentry CLI fails #17095
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
Conversation
…ils (#17082) Sentry CLI's `cli.releases.uploadSourceMaps` method previously never rejected when the actual CLI binary execution exited with an error code. In CLI 2.49.0 (and 2.50.0) I added a new live mode `rejectOnError` which continues to pipe stdio to the process (the remix SDKs' upload script) but now also rejects on error. This PR 1. bumps Sentry CLI, 2. configures the CLI to actually reject now but 3. also catches the rejection and logs a message. I decided to still continue with the script because we should still delete source maps. Otherwise, we risk deploying them when users expect them to be deleted. (i.e. fail silently but correctly :D)
Co-authored-by: Andrei <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Sentry Source Maps Upload Failures and Data Loss
The createRelease.js
script now silently fails to upload source maps and prematurely deletes local files.
- An invalid
live: 'rejectOnError'
option passed tosentry.releases.uploadSourceMaps
causes the Sentry CLI to error, preventing uploads. This error is silently swallowed by a newtry...catch
block. - Due to errors from
uploadSourceMaps
andfinalize
being swallowed,deleteSourcemaps
now executes unconditionally when--deleteAfterUpload
is used. This deletes local source maps even if they were never successfully uploaded or finalized, leading to data loss and preventing debugging or re-uploads.
packages/remix/scripts/createRelease.js#L29-L54
sentry-javascript/packages/remix/scripts/createRelease.js
Lines 29 to 54 in 003c253
try { | |
await sentry.releases.uploadSourceMaps(release, { | |
urlPrefix: URL_PREFIX, | |
include: [BUILD_PATH], | |
useArtifactBundle: !argv.disableDebugIds, | |
live: 'rejectOnError', | |
}); | |
} catch { | |
console.warn('[sentry] Failed to upload sourcemaps.'); | |
} | |
try { | |
await sentry.releases.finalize(release); | |
} catch { | |
console.warn('[sentry] Failed to finalize release.'); | |
} | |
if (argv.deleteAfterUpload) { | |
try { | |
deleteSourcemaps(BUILD_PATH); | |
} catch (error) { | |
console.warn(`[sentry] Failed to delete sourcemaps in build directory: ${BUILD_PATH}`); | |
console.error(error); | |
} | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
Small cleanup forward-port to `develop` (see #17095 (comment).) Co-authored-by: Andrei <[email protected]>
Backport of #17082