feat(wasm-debug-files): Add prepare command for WASM debug setup - #1572
d2anamaria wants to merge 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const name = basename(wasmPath).replace(WASM_EXTENSION, ""); | ||
| const fileName = `${name}${COMPANION_SUFFIX}`; | ||
| return join(outDir ?? dirname(wasmPath), fileName); | ||
| } |
There was a problem hiding this comment.
--out-dir overwrites same-named companions
High Severity
--out-dir names every companion from the module basename alone, so a recursive scan that hits two app.wasm files writes the same app.debug.wasm twice. The first module is stripped in place, then its companion is overwritten, so that DWARF is gone from both the deployable and disk and later runs cannot recover it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8c8468e. Configure here.
There was a problem hiding this comment.
not sure what’s the best solution for this.
- remove flag, companions will always sit in the deploy directory
- name by build_id, filename stops being human recognizable
- another?
There was a problem hiding this comment.
name by build_id, filename stops being human recognizable
I think this makes the most sense.
| } | ||
|
|
||
| return { | ||
| hint: `Uploaded ${uploads.length} debug companion(s) to ${params.org}/${params.project}`, |
There was a problem hiding this comment.
Source bundles counted as companions
Low Severity
filesUploaded is uploads.length minus failures, and --include-sources appends source-bundle DIFs into that same list. Human output then reports those extras as companions, so one companion plus one source bundle is shown as Uploaded 2 companions.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8c8468e. Configure here.
|
A deploy |
Sounds like we should, yes. Also a simple warning in this case sounds like the wrong action as we are essentially uploading broken stuff? |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
344b5b7 to
cf5ae11
Compare
0fc4d8f to
a408240
Compare
- Add isCodeSection and isExternalDebugInfoSection predicates - Add decodeExternalDebugInfo on top of the private byte-vector decoder - Name the code section id instead of repeating 10 in SECTION_ORDER - Add debugIdFromBuildId, rendering a hex build id as a dashed debug id - Make readVarUint32, writeVarUint32, and VarUint32 module-private - Drop the LEB128 test block that only covered those exports Co-authored-by: Cursor <cursoragent@cursor.com>
- Add prepareWasmFile, classifying a module and splitting it when it has DWARF - Call the shared splitWasm instead of a second copy of the split logic - Read sections through binary predicates, not raw section constants - Own the build-id file I/O here, keeping build-id.ts free of disk access - Repair a pair split by another tool by reconciling one shared build id - Pin byte-for-byte parity with Symbolicator wasm-split via golden vectors Co-authored-by: Cursor <cursoragent@cursor.com>
- Stamp build ids through splitWasm instead of rebuilding the section list - Drop a redundant parseSections per scanned module and per repaired companion - Move readSourceFile into read-file.ts, shared with debug-files upload - Add wait.ts, replacing the resolveWaitMode copy each upload path carried - Type the wait flags structurally so neither command imports the other's flags Co-authored-by: Cursor <cursoragent@cursor.com>
- Add debug-files prepare, registering it in the debug-files route map - Render one key-value table per module through the markdown pipeline - Keep each warning and recommendation in its own module's table - Colour the skip label and warnings with colorTag, never raw chalk - Print debug quality as the JSON payload spells it, dropping the remap - Gate uploads on --require-dwarf before any file is sent - Refresh the generated skill reference and command docs Co-authored-by: Cursor <cursoragent@cursor.com>
- Guard the inspectWasm build_id survey with ??= so a later malformed build_id section cannot erase an id already found, matching buildIdFromSections and the Rust tool's find_map - Intersect PrepareFlags and UploadFlags with WaitFlags instead of restating --wait / --wait-for in each command - Drop the orphaned resolveWaitMode JSDoc left behind in prepare.ts, which was nesting into the resolveBuildId comment block
5f68620 to
9025888
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 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 9025888. Configure here.
| isWasmPath(path) && | ||
| !isDebugCompanionPath(path) && | ||
| !isIgnored(path, ignoreMatchers, paths) | ||
| ); |
There was a problem hiding this comment.
Scan splits custom-named companions
High Severity
Directory scans only skip files named *.debug.wasm. A companion with any other name is treated as a deployable module, so prepare strips its DWARF in place. That destroys the pair repairUnpairedCompanion was added to fix, and later upload can send a companion that no longer contains debug info.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9025888. Configure here.
There was a problem hiding this comment.
This would require another directory pass to parse every candidate and collect which paths are pointed at by someone else's external_debug_info, and exclude them from the next scan. Is it a too narrow edge case to add this extra pass&parse?
This solution only helps when the deployable is in the same scan and still carries its pointer, so custom-named companions sitting alone still can't be detected.
There was a problem hiding this comment.
Not fully familiar with the logic but why can we not build an in-memory tree as we do the first scan and use that later on?
| * @param sourcePath - Path the debug file references. | ||
| * @returns The contents, or `null` when the file is not available locally. | ||
| */ | ||
| export function readSourceFile(sourcePath: string): Uint8Array | null { |
There was a problem hiding this comment.
Why do we need a helper like this and why is it not using async or promisified versions?
There was a problem hiding this comment.
Not new, it was already a private helper in upload.ts. I moved it so the new command can reuse it instead of duplicating the try/catch. Sync because that requirement bubbles up from symbolic.
| isWasmPath(path) && | ||
| !isDebugCompanionPath(path) && | ||
| !isIgnored(path, ignoreMatchers, paths) | ||
| ); |
There was a problem hiding this comment.
Not fully familiar with the logic but why can we not build an in-memory tree as we do the first scan and use that later on?
- reuse shared readSourceFile in bundle-sources instead of an inline readFileSync try/catch - build the prepare report by pushing module sections into the summary array instead of spread/flatMap intermediates - name all thirteen non-custom wasm section ids as constants in SECTION_ORDER - trim the prepare docs fragment: drop the wasm-split parity note and shorten the idempotency line
- Move buildIgnoreMatcher into lib/scan/ignore.ts - Reuse it in debug-files prepare and sourcemap commands
loewenheim
left a comment
loewenheim
left a comment
There was a problem hiding this comment.
On a high level I think this command is misnamed, on two counts:
- It doesn't apply to debug files in general, only wasm.
- "Prepare" doesn't sound like it uploads files, it sounds like something you call before you upload them.
I'm just spitballing, but I could see this as a --split-wasm flag on debug-files upload that splits wasm files with default options (and you can either add options to customize the wasm-split behavior or tell users to run wasm-split manually if they need to customize it).
I agree the name is a bit misleading, but I'd fix the name rather than folding it into upload. This command rewrites your build artifacts, while upload is read-only. Carrying the wasm-split flags over would bloat upload with options that do nothing unless you're uploading wasm. |
|
Yeah, that's fair both on the read-only point and the options point. Again, just a thought: maybe the new command could be folded into |
3 participants
Problem
Preparing WebAssembly for Sentry takes two tools today. You run
wasm-splitto inject abuild_idand pull DWARF out into a companion file, then runsentry debug-files upload --type wasmon that companion. Nothing tells you when a module was built without usable debug info, so the mistake surfaces later as an unsymbolicated stack trace.Solution
sentry debug-files prepare <path>...does both steps in one command.It scans the given files and directories for
.wasmmodules, splits the ones carrying inline DWARF, and uploads the companions. Org and project are auto-detected from DSN, env vars, or config defaults.--dry-runand--no-uploadneed no credentials.What it does per module
For a module with inline DWARF:
build_idif it has none.*.debug.wasmcompanion keeping every section, including Code and DWARF..debug_*sections from the deployable module, in place.external_debug_info.The deployable keeps its original path, so your build artifact does not move. Both files carry the same
build_id, which is how Sentry matches a stack frame to its debug file. The companion keeps the Code section because DWARF addresses are relative to it.A module that cannot be split is still stamped with a
build_id, then reported with a warning. Stamping matters: without abuild_ida module can never be symbolicated, not even from a debug file uploaded later. Nothing is uploaded for it.build_idalready exists--dry-runpreviewWarnings
A module without usable debug info does not fail the run unless
--require-dwarfis set. The warning says why it was skipped:no line-level symbolication (name/symtab only)no debug information; rebuild with DWARF (Emscripten -g, wasm-pack dwarf-debug-info)already stripped (build_id present, no debug sections); splitting would produce a useless companionhas external_debug_info but no local companion with matching build_idRe-running is safe. An already-prepared pair is detected and left alone rather than overwritten with an empty companion, and a module keeps the
build_idit was stamped with on the first run.Options
--dry-run--no-upload--require-dwarf--out-dir <DIR>--strip-namesnamesection from split deployables; the companion keeps it--build-id <UUID>--include-sources--wait/--wait-for <SECS>--ignore <GLOB>/--ignore-file <FILE>--require-dwarfis the CI guard, and it runs before anything is uploaded, so a build missing debug info fails without pushing files first. A module pointing at an external companion counts as having DWARF, so a dangling pointer does not fail the build.--ignoreglobs are relative to the tree you point at:--ignore 'vendor/**'withprepare ./distmeans./dist/vendor.Scanning rules
Directories are walked recursively.
*.debug.wasmfiles are skipped, since they are outputs of an earlier run. Naming a non-.wasmfile directly is an error; a directory with no modules is just an empty scan.Automation
--jsonreports the outcome per module, so a build script can act on the classification instead of grepping logs:{ "org": "my-org", "project": "my-project", "uploaded": true, "filesUploaded": 1, "modules": [ { "path": "dist/app.wasm", "action": "split", "quality": "dwarf", "buildId": "…", "companion": "dist/app.debug.wasm" } ] }The command exits non-zero when
--require-dwarffails, and when a companion fails server-side processing under--wait.Relationship to wasm-split
The split follows the same algorithm as Symbolicator's
wasm-split, and custom sections follow the WebAssembly tool conventions, so companions are meant to be interchangeable in Sentry. It is a reimplementation, not a shared codepath:wasm-splitis a binary-only Rust crate, so this ships its own minimal module reader. Only the module envelope is parsed and section payloads are re-emitted byte for byte, so sections the reader does not know about survive untouched.Limitations
build_idbut no debug file. Rebuild with DWARF and re-run to get line-level frames; thebuild_idis preserved.external_debug_inforecords the companion filename only. There is no equivalent ofwasm-split --external-dwarf-url. Sentry resolves bybuild_id, so this does not affect symbolication.--out-dir, re-running without the same--out-dirwill not find the companion and reports a dangling reference.