feat(remix): Add debugid injection and map deletion to sourcemaps script #8814
Conversation
size-limit report 📦
|
@lforst should we expose debug id injection via the js sdk of sentry-cli? |
packages/remix/package.json
Outdated
| @@ -27,7 +27,7 @@ | |||
| "access": "public" | |||
| }, | |||
| "dependencies": { | |||
| "@sentry/cli": "2.2.0", | |||
| "@sentry/cli": "2.20.4", | |||
There was a problem hiding this comment.
Let's bump this to 2.20.5 instead. It contained a pretty critical performance fix.
@AbhiPrasad I am assuming you're talking about the JS API of Sentry CLI. While the Remix SDK is the only location we need something like this I'd rather not add to the CLI API just now. If something similar comes up one or two more times then we should probably add it. |
Yup I'm talking about the JS API. I think it's fair to not prio it then, there is actually quite a bit of improvements we can make to the js API overall now that I'm looking at the code. |
onurtemizkan
force-pushed
the
onur/remix-sourcemap-upload-improvements
branch
from
5593bd3 to
d45ca37
Compare
| const path = require('path'); | ||
|
|
||
| function deleteSourcemaps(buildPath) { | ||
| console.info(`Deleting sourcemaps from ${buildPath}`); |
There was a problem hiding this comment.
Let's append the logs with [sentry] so it's clear where this is coming from.
onurtemizkan
force-pushed
the
onur/remix-sourcemap-upload-improvements
branch
from
4d03001 to
0efd98d
Compare
In general the event looks fine to me. Admittedly,
looks a bit weird... But I think this comes from Symbolicator and not the SDK. |
|
|
||
| function injectDebugId(buildPath) { | ||
| try { | ||
| execSync(`node ./node_modules/@sentry/cli/bin/sentry-cli sourcemaps inject ${buildPath}`); |
There was a problem hiding this comment.
I can imagine some environments (e.g. monorepos) where this path lookup will not succeed.
The @sentry/cli package exports a function called getPath that we can use to get the binary path. Wdyt about using it here?
Additionally, I wouldn't call the node executable but the binary directly.
There was a problem hiding this comment.
Makes sense, updated 👍
| function deleteSourcemaps(buildPath) { | ||
| console.info(`[sentry] Deleting sourcemaps from ${buildPath}`); | ||
|
|
||
| const files = fs.readdirSync(buildPath); |
There was a problem hiding this comment.
One more q: Is the output in the build folder always only one flat layer or could there be folders inside? Otherwise, we might need to recurse here, and/or use something like the glob library.
There was a problem hiding this comment.
Yes, it was missing out the files in _shared folder. Updated 👍
onurtemizkan
force-pushed
the
onur/remix-sourcemap-upload-improvements
branch
from
e54acce to
22e5b7a
Compare
Updated
sentry-upload-sourcemapsscript:sentry-clito:2.20.4sentry-cli(as this feature is not available in JS SDK ofsentry-cliyet)