Skip to content

feat(debug-files): add upload command (core)#1139

Merged
BYK merged 1 commit into
mainfrom
byk/feat/debug-files-upload
Jun 24, 2026
Merged

feat(debug-files): add upload command (core)#1139
BYK merged 1 commit into
mainfrom
byk/feat/debug-files-upload

Conversation

@BYK

@BYK BYK commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

Ports the legacy sentry-cli debug-files upload to the TypeScript CLI, scoped to Tier A core — the path that covers the ~90% real-world case (ELF, dSYM/Mach-O, PE/PDB, WASM, Breakpad, Portable PDB). Reuses the existing DIF chunk-upload + assemble protocol (projects/{org}/{project}/files/difs/assemble/) already used by proguard upload / dart-symbol-map upload, and the @sentry/symbolic WASM parser already used by debug-files check / bundle-sources.

What's included

  • sentry debug-files upload <path>... — recursively scans files/directories, parses each candidate in-process, filters, and uploads matching files.
  • Filters (mirror legacy filter_features): --type/--id (repeatable), --require-all, --no-debug/--no-unwind/--no-sources. --no-debug drops both debug and symtab, matching legacy semantics.
  • --include-sources — builds and attaches an in-memory source bundle per file with debug info.
  • --no-upload dry-run (no credentials needed, like proguard upload).
  • --wait / --wait-for <secs> — block on server-side processing; collect error states and exit non-zero.
  • --json output (auto-injected by buildCommand).

Design notes

  • Per-file upload, not per-slice. Each scanned file is chunked as raw bytes once and assembled with the primary object's debug_id (first object with debug info, falling back to the first). The server re-parses and indexes every slice itself; debug_id is advisory (omitted when nil/absent).
  • Two completion modes: no-wait stops once the server holds every chunk; wait polls until every file is terminal (ok/error). Unlike proguard/dart, an error entry is collected (not thrown) so the command can report all failures and set the exit code.
  • Pure, property-tested filters (src/lib/dif/scan.ts) separated from the scan/parse I/O. --id and --require-all match ignoring the PE/PDB age suffix via a shared debugIdMatches helper.

Deferred to follow-ups (noted in --help)

ZIP scanning (--no-zips), --symbol-maps (BCSymbolMap resolution — needs Apple dsymutil), --il2cpp-mapping line mappings, --derived-data. A full Tier A–D feasibility roadmap is captured in the plan.

Testing

  • test/lib/dif/scan.property.test.ts — property tests for objectPassesFilters, normalizeDebugId, debugIdMatches, buildDifFilters.
  • test/lib/api/debug-files.test.ts — assemble body shape, missing-chunk upload, no-wait vs wait stop conditions, error collection, timeout.
  • test/commands/debug-files/upload.test.ts — Breakpad text fixtures: dir scanning, --type/--id filters, --no-upload, --require-all miss, wait-mode error exit, validation.

All new tests pass; pnpm run lint and pnpm run typecheck are clean.

@github-actions github-actions Bot added the risk: high PR risk score: high label Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-1139/

Built to branch gh-pages at 2026-06-24 19:07 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Comment thread src/commands/debug-files/upload.ts
Comment thread src/commands/debug-files/upload.ts Outdated
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

✅ Patch coverage is 84.81%. Project has 5101 uncovered lines.
✅ Project coverage is 81.47%. Comparing base (base) to head (head).

Files with missing lines (3)
File Patch % Lines
src/commands/debug-files/upload.ts 83.33% ⚠️ 17 Missing and 8 partials
src/lib/dif/scan.ts 79.21% ⚠️ 21 Missing and 4 partials
src/lib/api/debug-files.ts 95.52% ⚠️ 3 Missing and 2 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    81.44%    81.47%    +0.03%
==========================================
  Files          393       396        +3
  Lines        27254     27525      +271
  Branches     17699     17863      +164
==========================================
+ Hits         22196     22424      +228
- Misses        5058      5101       +43
- Partials      1847      1862       +15

Generated by Codecov Action

Comment thread src/lib/dif/scan.ts
@BYK BYK force-pushed the byk/feat/debug-files-upload branch from 9f0c2a9 to 1567eb4 Compare June 24, 2026 18:27
@BYK

BYK commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Self-review findings (all addressed)

A critical self-review was completed via subagent. No Critical issues found. The following were surfaced and fixed in the current push (1567eb42c):

High (fixed)

  • Symlink-cycle hang (collectFiles): added visited-realpath set tracking to break infinite recursion on cyclic directory symlinks (dSYM frameworks, etc.).
  • Memory: full read before format check (prepareDifs): added header-sized read (4 KB) for peekFormat; only fully read files whose format is recognised. Large non-DIF files (videos, archives) cost only the header I/O.

Medium (fixed)

  • --require-all fat-binary false miss: missingRequestedIds now scans all matched objects (both slices of a fat Mach-O), not just the per-file primary debugId.
  • not_found silently treated as success: the deadline-break in no-wait mode was log.debug (hidden); upgraded to log.warn, and the command now treats not_found as a terminal failure (exit 1 + descriptive hint).
  • Silent nonexistent path: scanPaths now throws ValidationError for an explicit path that does not exist (matches proguard upload UX).

Low (acknowledged, not changed)

  • --require-all enforce-after-upload: intentional — matching legacy behavior. Not a correctness issue.
  • Nil-id check is hyphen-form only: the parser always emits canonical hyphenated UUIDs, so the non-hyphenated case can't arise in practice.
  • Content hashed twice for dedup: minor — the chunker computes its own checksum internally. Negligible for actual DIF file sizes.

Comment thread src/commands/debug-files/upload.ts
Comment thread src/lib/api/debug-files.ts Outdated
Comment thread src/commands/debug-files/upload.ts
@BYK BYK force-pushed the byk/feat/debug-files-upload branch from 1567eb4 to db88b53 Compare June 24, 2026 18:50

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit db88b53. Configure here.

Comment thread src/lib/dif/scan.ts
Comment thread src/lib/api/debug-files.ts Outdated
Port the legacy `sentry-cli debug-files upload` to the TypeScript CLI,
scoped to the Tier A core path that covers the ~90% real-world case.

- New `upload` command: recursively scans files/directories, parses each
  candidate via the bundled `@sentry/symbolic` WASM module, filters, and
  uploads matching files via the DIF chunk-upload + assemble protocol.
- Filters: `--type`/`--id` (repeatable), `--require-all`, and
  `--no-debug`/`--no-unwind`/`--no-sources` (mirrors legacy feature filtering).
- `--include-sources` attaches an in-memory source bundle per file.
- `--no-upload` dry-run (no credentials); `--wait`/`--wait-for` block on
  server-side processing and exit non-zero on failures.
- `src/lib/api/debug-files.ts`: `uploadDebugFiles` batches all files into one
  assemble request keyed by SHA-1 checksum, with no-wait (chunks held) and
  wait (terminal ok/error) completion modes.
- `src/lib/dif/scan.ts`: pure, property-tested filter predicates plus the
  filesystem scan/parse pipeline.

Deferred to follow-ups (noted in --help): ZIP scanning, `--symbol-maps`,
`--il2cpp-mapping` line mappings, and `--derived-data`.
@BYK BYK force-pushed the byk/feat/debug-files-upload branch from db88b53 to 71a0b36 Compare June 24, 2026 19:06
@BYK BYK merged commit 6acb9aa into main Jun 24, 2026
60 of 66 checks passed
@BYK BYK deleted the byk/feat/debug-files-upload branch June 24, 2026 19:39
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
BYK added a commit that referenced this pull request Jun 25, 2026
Follow-up to #1139 covering the "outer upload envelope" parity with the
legacy `dif_upload`:

- Parse the server-advertised `maxFileSize` / `maxWait` from chunk-upload
  options (both optional; absent/0 means no cap).
- Skip files larger than the server's `maxFileSize` before assembling
  (with a warning); `prepareDifs` also gains a scan-time size gate using
  the 2 GiB default so dry-runs report realistic skips.
- Clamp the processing wait to the server's `maxWait` in wait mode.
- Add `--derived-data` to also scan Xcode's DerivedData folder on macOS
  (existence-gated so the stricter scanPaths check is not tripped).

Tests cover the schema's optional fields, the size gate (scan + upload),
the wait clamp, and the --derived-data branches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant