feat(wasm-debug-files): Add prepare command for WASM debug setup - #1572
d2anamaria wants to merge 11 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.
|
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
- 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?
| 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 |
- name companions <stem>.<build_id>.debug.wasm instead of <stem>.debug.wasm - resolve build id before naming so unstamped modules get unique filenames - write external_debug_info as a path relative to the module, not basename only - dry-run reports exact path when id is known, <build-id> placeholder otherwise - update prepare help text and docs to match the new companion naming
| if ( | ||
| buildIdsMatch( | ||
| await readCompanionBuildId(expectedCompanion), | ||
| inspection.buildId | ||
| ) | ||
| ) { | ||
| return { companion: expectedCompanion, quality: inspection.quality }; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Bug: findExistingCompanion returns the main file's quality, not the companion's. This bypasses the --require-dwarf check, causing a DWARF-less companion file to be uploaded.
Severity: MEDIUM
Suggested Fix
The findExistingCompanion function should inspect the companion file and return its actual debug quality, rather than propagating the quality from the main WASM file's inspection result. This ensures that the subsequent --require-dwarf check correctly identifies if the companion lacks necessary debug information.
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/cli/src/lib/wasm/prepare.ts#L394-L402
Potential issue: When preparing a WASM file that has no DWARF info but has a `build_id`
and an existing companion file (which also lacks DWARF), the `--require-dwarf` flag is
incorrectly ignored. The `findExistingCompanion` function mistakenly returns the quality
of the main file ("none") instead of inspecting the companion file's quality. This leads
to the result being marked as `"already-prepared"` with `quality: "none"`. The
subsequent `lacksDwarf` check then incorrectly passes because the action is not
`"skipped"`, allowing a DWARF-less companion file to be uploaded, which will cause
symbolication to fail silently.
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
sentry wasm-splitsplits one module and stops there. This command is the pipeline around it: itscans directories, classifies each module's debug quality, skips what it should not touch, and
uploads the companions. The split itself is the same
splitWasmcall.Elsewhere the CLI reads debug files through
@sentry/symbolicthat wrapper (only reads).This command rewrites modules: it injects
build_id, strips.debug_*, and addsexternal_debug_info. So it parses sections directly.That is also why the two disagree on
build_id. This command follows the tool convention;symbolicreads it one byte shifted. The id sent here is advisory — the server re-parses the moduleand stores its own key — so nothing breaks today. See
getsentry/symbolic#1069.
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.preparedoes not exposewasm-split's--external-dwarf-url; reach forsentry wasm-splitif you need a custom 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.