feat(proguard): add 'proguard upload' command (chunk-upload of R8/ProGuard mappings)#1074
Conversation
…Guard mappings)
Implement `sentry proguard upload <path>...` to upload Android ProGuard/R8
mapping files via the DIF chunk-upload + assemble protocol.
## Changes
- Extract shared chunk-upload infrastructure from `sourcemaps.ts` into
`chunk-upload.ts` (schemas, types, codec selection, chunking, hashing,
upload, and assembly polling)
- Add `proguard.ts` API module for building proguard ZIP bundles
(`proguard/<uuid>.txt` entries, no manifest) and uploading via
the DIF assemble endpoint (`projects/{org}/{project}/files/difs/assemble/`)
- Add `proguard upload` command with flags:
--uuid (force UUID), --no-upload (dry-run), --require-one,
--no-reprocessing
- Refactor `sourcemaps.ts` to import from shared `chunk-upload.ts`
with backward-compatible re-exports
Closes #1053
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Results 📊❌ Patch coverage is 40.74%. Project has 4846 uncovered lines. Files with missing lines (4)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 81.54% 81.36% -0.18%
==========================================
Files 369 372 +3
Lines 25845 25992 +147
Branches 16888 16949 +61
==========================================
+ Hits 21074 21146 +72
- Misses 4771 4846 +75
- Partials 1770 1772 +2Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 53a8de3. Configure here.
…file assemble, drop --no-reprocessing - Upload raw mapping bytes directly (no ZIP wrapping, no SYSB header) matching the legacy sentry-cli DIF protocol - Each mapping gets its own entry in the assemble request body, keyed by individual checksum with correct name/chunks - Add hashBuffer() and uploadMissingBufferChunks() to chunk-upload.ts for in-memory buffer chunking - Remove --no-reprocessing flag (not in legacy CLI for proguard) - Extract checkAssembleResponse() helper to reduce cognitive complexity - Update tests for new raw-byte approach
…entical content, import shared constants - Set auth:false so --no-upload works without credentials - Guard against empty mapping files with ValidationError - Deduplicate mappings with identical content (same UUID) with warning - Import ASSEMBLE_POLL_INTERVAL_MS/ASSEMBLE_MAX_WAIT_MS from chunk-upload.ts - Remove unnecessary Buffer.from() copy in uploadBufferChunk - Add logging to silent catch in uploadBufferChunk - Extract deduplicateMappings() to keep func() under complexity limit
| for (const cm of chunkedMappings) { | ||
| await uploadMissingBufferChunks({ | ||
| chunks: cm.chunks, | ||
| missingChecksums, | ||
| content: cm.mapping.content, | ||
| serverOptions, | ||
| encoding, | ||
| regionUrl, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: The loop for uploading ProGuard mapping chunks is sequential (for...await) instead of parallel, slowing down uploads of multiple mapping files.
Severity: MEDIUM
Suggested Fix
To fix the performance issue, the sequential for...await loop should be replaced with a Promise.all() call. This would involve mapping over chunkedMappings and calling uploadMissingBufferChunks for each, allowing all uploads to run concurrently up to the concurrency limit. Example: await Promise.all(chunkedMappings.map((cm) => uploadMissingBufferChunks({...})));
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: src/lib/api/proguard.ts#L193-L202
Potential issue: The code at `src/lib/api/proguard.ts` uses a sequential `for...await`
loop to upload chunks for multiple ProGuard mappings. This contradicts a comment stating
the uploads happen "in parallel across all mappings". Each `uploadMissingBufferChunks`
call completes before the next one begins, processing each mapping file serially. This
underutilizes the available concurrency, leading to a performance degradation when a
user uploads multiple mapping files simultaneously. The total upload time is
unnecessarily extended because the uploads for different files do not run in parallel as
intended.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| const { data: pollResult } = await apiRequestToRegion<DifAssembleResponse>( | ||
| regionUrl, |
There was a problem hiding this comment.
Missing chunks reported during DIF assembly polling are never re-uploaded, leading to timeout
In pollDifAssembly, only allDone is destructured from checkAssembleResponse; the missingChecksums set is discarded. uploadProguardMappings uploads missing chunks exactly once (step 5) before entering the poll loop and never retries. If the server reports missingChunks in any poll response (e.g. a partially-failed initial upload, chunk eviction, or transient storage delay), those chunks are never re-uploaded; the loop spins until the 300 s deadline and then throws an ApiError(408) timeout instead of completing the upload. This is a robustness/correctness gap rather than a guaranteed failure, since it depends on the server reporting missing chunks mid-poll.
Evidence
checkAssembleResponsereturns{ allDone, missingChecksums }, populatingmissingChecksumsfrom each entry'smissingChunkswhenever itsstateis notok/created/error.- In
pollDifAssemblythe poll loop destructures onlyallDone:const { allDone } = checkAssembleResponse(pollResult, checksums, endpoint)—missingChecksumsis dropped, and there is nouploadMissingBufferChunkscall inside the loop. uploadProguardMappingsstep 5 callsuploadMissingBufferChunksonce before invokingpollDifAssembly; no retry path exists if the server still reports chunks missing on a later poll.- On deadline expiry (
ASSEMBLE_MAX_WAIT_MS= 300 000 ms) the function throwsApiError(... 408 ...)rather than re-sending the chunks.
Also found at 1 additional location
src/lib/api/chunk-upload.ts:243-246
Identified by Warden find-bugs · 9S8-2G9
1 participant
Summary
Implements
sentry proguard upload <path>...to upload Android ProGuard/R8 mapping files via the DIF chunk-upload protocol. Part of the parity effort tracked in #600.Closes #1053
Changes
sourcemaps.tsinto newsrc/lib/api/chunk-upload.ts— schemas, types, codec selection, chunking, hashing, chunk upload, and assembly polling are now shared between sourcemap and proguard uploadshashBuffer,uploadMissingBufferChunks) — ProGuard mappings are chunked as raw bytes (no ZIP wrapping), matching the legacysentry-cliDIF protocolsrc/lib/api/proguard.ts) — each mapping file's raw bytes are independently chunked and assembled via the DIF endpoint (projects/{org}/{project}/files/difs/assemble/), with all mappings in a single assemble request keyed by individual checksumssentry proguard uploadcommand (src/commands/proguard/upload.ts) with flags:--uuid— force a specific UUID instead of computing from content (single file only)--no-upload— dry-run mode: compute and print UUIDs without uploading (no auth needed)--require-one— error if no mapping files providedsourcemaps.tsto import from sharedchunk-upload.tswith backward-compatible re-exportsProtocol Notes
The DIF upload protocol differs from artifact bundles (sourcemaps) in three ways:
projects/{org}/{project}/files/difs/assemble/(per-project, not per-org)This matches the legacy
sentry-cli(Rust) implementation byte-for-byte.