Skip to content

feat(debug-files): honor server upload limits and add --derived-data#1140

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

feat(debug-files): honor server upload limits and add --derived-data#1140
BYK merged 1 commit into
mainfrom
byk/feat/debug-files-upload-limits

Conversation

@BYK

@BYK BYK commented Jun 25, 2026

Copy link
Copy Markdown
Member

Follow-up to #1139. Scopes the outer upload envelope of debug-files upload to parity with the legacy dif_upload — honoring server-advertised limits and adding Xcode DerivedData scanning. No WASM changes.

What's included

  • Server maxFileSize enforcement — parse the optional maxFileSize from chunk-upload options; files exceeding it are dropped (with a warning) before assembling. prepareDifs also gains a scan-time size gate using the 2 GiB DEFAULT_MAX_DIF_SIZE default so --no-upload dry-runs report realistic skips without needing server options.
  • Server maxWait clamp — in wait mode, the processing wait is clamped to min(requested, server.maxWait). No-wait mode leaves its hang-guard deadline untouched.
  • --derived-data — additionally scans ~/Library/Developer/Xcode/DerivedData on macOS. Existence-gated (and a no-op with a warning on non-macOS), so the stricter scanPaths existence check from feat(debug-files): add upload command (core) #1139 is never tripped by an absent folder.

Both server fields are optional in the schema (maxFileSize / maxWait); a server response that omits them parses unchanged (backward compatible), and 0 means "no cap" (legacy semantics).

Testing

  • chunk-upload.test.ts — schema parses with/without the new optional fields.
  • debug-files.test.ts — oversized files dropped before assemble; all-oversized returns [] with no request; wait clamped to server maxWait (reflected in the timeout detail); no clamp when server advertises 0.
  • scan.test.tsprepareDifs size gate keeps/skips by limit.
  • upload.test.ts--derived-data is additive with explicit paths; on non-macOS, --derived-data alone exits non-zero (no scan targets).

53 tests pass across the touched suites; full repo lint + typecheck clean.

Deferred to follow-ups

filter_bad_sources + il2cpp sources (next PR), fix_pdb_ages (after that). Blocked on upstream WASM/tooling: --il2cpp-mapping line mappings (ObjectLineMapping), --symbol-maps (Apple dsymutil).

@github-actions github-actions Bot added the risk: medium PR risk score: medium label Jun 25, 2026
@github-actions

github-actions Bot commented Jun 25, 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-1140/

Built to branch gh-pages at 2026-06-25 13:10 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

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

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

✅ Patch coverage is 86.46%. Project has 5107 uncovered lines.
✅ Project coverage is 81.49%. Comparing base (base) to head (head).

Files with missing lines (3)
File Patch % Lines
src/commands/debug-files/upload.ts 84.21% ⚠️ 6 Missing and 2 partials
src/lib/dif/scan.ts 84.85% ⚠️ 5 Missing and 2 partials
src/lib/api/debug-files.ts 91.67% ⚠️ 2 Missing and 2 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    81.46%    81.49%    +0.03%
==========================================
  Files          396       396         —
  Lines        27525     27588       +63
  Branches     17863     17912       +49
==========================================
+ Hits         22422     22481       +59
- Misses        5103      5107        +4
- Partials      1861      1865        +4

Generated by Codecov Action

Comment thread src/lib/api/debug-files.ts Outdated
Comment thread src/lib/dif/scan.ts Outdated
@BYK BYK force-pushed the byk/feat/debug-files-upload-limits branch from 249c931 to aacabfb Compare June 25, 2026 10:29
Comment thread src/lib/dif/scan.ts
Comment thread src/commands/debug-files/upload.ts
Comment thread src/commands/debug-files/upload.ts Outdated
@BYK BYK force-pushed the byk/feat/debug-files-upload-limits branch 2 times, most recently from 3f5f100 to 775acd0 Compare June 25, 2026 11:15
Comment thread src/commands/debug-files/upload.ts Outdated
@BYK BYK force-pushed the byk/feat/debug-files-upload-limits branch from 775acd0 to 8054816 Compare June 25, 2026 11:43
Comment thread src/commands/debug-files/upload.ts Outdated
Comment thread src/commands/debug-files/upload.ts Outdated
@BYK BYK force-pushed the byk/feat/debug-files-upload-limits branch from 8054816 to e27e349 Compare June 25, 2026 11:56
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-limits branch from e27e349 to faea897 Compare June 25, 2026 12:13
@github-actions github-actions Bot added risk: high PR risk score: high and removed risk: medium PR risk score: medium labels Jun 25, 2026
@BYK BYK force-pushed the byk/feat/debug-files-upload-limits branch from faea897 to 38245f8 Compare June 25, 2026 12:19
Comment thread src/lib/dif/scan.ts
@BYK BYK force-pushed the byk/feat/debug-files-upload-limits branch from 38245f8 to b38b62f Compare June 25, 2026 12:52

@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 b38b62f. Configure here.

Comment thread src/commands/debug-files/upload.ts Outdated
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 BYK force-pushed the byk/feat/debug-files-upload-limits branch from b38b62f to 751cf0b Compare June 25, 2026 13:10

@sentry-warden sentry-warden Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Assembly deadline runs down during chunk upload, causing false timeout when server maxWait is tight

In debug-files.ts, deadline = Date.now() + effectiveMaxWaitMs is stamped before the initial postAssemble + uploadMissing calls, so slow chunk delivery consumes the server-clamped budget. If upload takes longer than effectiveMaxWaitMs, the while loop's first Date.now() >= deadline check fires and throws ApiError(408) even though all chunks were successfully delivered and the server will complete assembly — producing a false failure on builds with large DIFs and a low server maxWait.

Evidence
  • effectiveMaxWaitMs is clamped to serverOptions.maxWait * 1000 when wait=true (debug-files.ts:415-417).
  • deadline = Date.now() + effectiveMaxWaitMs is set at line 418, before postAssemble (line 419) and uploadMissingBufferChunks (lines 421-427).
  • The while loop checks Date.now() >= deadline at line 430 as its first action; if chunk upload consumed the budget, it immediately throws ApiError(408, 'Assembly did not complete within Xs').
  • By contrast, pollAssembly in chunk-upload.ts sets its deadline only after chunks are already uploaded by the caller, so artifact-bundle uploads don't have this race.
  • The server has received every chunk and will continue processing regardless; only the client reports failure.

Identified by Warden find-bugs

Comment on lines 594 to 596
const difs = dedupeDifs(
buildDifList(prepared, Boolean(flags["include-sources"]))
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Partial size-drop in filterBySize (e.g. oversized source bundles) silently exits 0

In uploadDebugFiles, filterBySize drops any DIF whose content.length exceeds effectiveMaxFileSize with only a log.warn. The accepted.length === 0 guard that raises ValidationError only fires when every file is dropped. When some files are accepted and only others are dropped — most notably the in-memory source bundles produced by --include-sources, which bypass the scan-time prepareDifs size gate — the dropped files never enter chunked, so buildResults produces no result entry for them. doUpload only flags results with state === "error" || "not_found", so failures.length stays 0, setExitCode(1) is never called, and the command prints "Uploaded N debug file(s)" and exits 0 with no indication the source bundle was dropped.

Evidence
  • filterBySize (debug-files.ts:309-327) drops oversized DIFs with only log.warn and continue, returning the remaining subset.
  • The accepted.length === 0 guard (debug-files.ts:391) only throws ValidationError when all files are dropped; a partial drop passes through silently.
  • buildResults (debug-files.ts:288) maps only over chunked, built from accepted, so dropped files have no entry in the returned results.
  • --include-sources bundles are pushed by buildDifList after prepareDifs ran its scan-time gate, so they reach filterBySize as the only files that can be oversized in the real-upload path; if a bundle exceeds the limit while its parent DIF does not, it is dropped silently.
  • doUpload (upload.ts) computes failures only from state === "error" || "not_found"; with no entry for the dropped bundle, failures.length is 0 and setExitCode(1) is never invoked, so the command exits 0.

Identified by Warden find-bugs · 9LL-87A

@BYK BYK merged commit 20b469a into main Jun 25, 2026
68 of 78 checks passed
@BYK BYK deleted the byk/feat/debug-files-upload-limits branch June 25, 2026 16:16
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