From e23dbf1581c5430b0adc2122321c0009909b6e7d Mon Sep 17 00:00:00 2001 From: Ivan Tustanivskyi Date: Wed, 4 Nov 2026 11:29:22 +0200 Subject: [PATCH 1/3] feat(cli): Upload DXIL shader binaries as debug files `debug-files upload` recognises DXIL containers (the compiled shaders an Unreal Engine cook exports) and uploads them as `dxil` debug files, which Sentry uses to name the faulting shader of a GPU crash. The debug id is the compiler hash in the file name. When the Sentry Unreal plugin's `sentry-shader-manifest.tsv` sits next to the files, each shader's Aftermath hash and shader group are sent as the new `code_id` and `shader_group` assemble fields. Assemble requests are sent in batches of a thousand files for every debug file type: a shader set is around ten thousand files per build, and a single assemble request for all of them ran for minutes on the server. --- .../src/fragments/commands/debug-files.md | 4 + .../sentry-cli/references/debug-files.md | 2 +- .../cli/src/commands/debug-files/upload.ts | 10 +- packages/cli/src/lib/api/debug-files.ts | 77 ++++++--- packages/cli/src/lib/dif/scan.ts | 112 ++++++++++++- packages/cli/src/lib/dif/shader-manifest.ts | 99 ++++++++++++ .../a223507a29b50f7132d44901c2228311.dxil | Bin 0 -> 7672 bytes .../dif/shader/sentry-shader-manifest.tsv | 2 + packages/cli/test/lib/api/debug-files.test.ts | 81 ++++++++++ packages/cli/test/lib/dif/dxil.test.ts | 149 ++++++++++++++++++ .../cli/test/lib/dif/shader-manifest.test.ts | 93 +++++++++++ 11 files changed, 599 insertions(+), 30 deletions(-) create mode 100644 packages/cli/src/lib/dif/shader-manifest.ts create mode 100644 packages/cli/test/fixtures/dif/shader/a223507a29b50f7132d44901c2228311.dxil create mode 100644 packages/cli/test/fixtures/dif/shader/sentry-shader-manifest.tsv create mode 100644 packages/cli/test/lib/dif/dxil.test.ts create mode 100644 packages/cli/test/lib/dif/shader-manifest.test.ts diff --git a/apps/cli-docs/src/fragments/commands/debug-files.md b/apps/cli-docs/src/fragments/commands/debug-files.md index da8bcc3f7..898975730 100644 --- a/apps/cli-docs/src/fragments/commands/debug-files.md +++ b/apps/cli-docs/src/fragments/commands/debug-files.md @@ -54,6 +54,10 @@ files. Combine with `--include-sources` to also bundle the referenced C# source files. - Upload a JVM bundle separately via `sentry debug-files upload --type jvm`. +- Compiled shaders (`.dxil` files from an Unreal Engine cook) are uploaded as + `dxil` debug files, which let Sentry name the shader that caused a GPU crash. + If the Sentry Unreal plugin wrote a `sentry-shader-manifest.tsv` next to them, + the shader hash and group from that manifest are uploaded as well. - Supported JVM source file extensions: `.java`, `.kt`, `.scala`, `.sc`, `.groovy`, `.gvy`, `.gy`, `.gsh`, `.clj`, `.cljc` - Build output directories (`build/`, `target/`, `out/`, `bin/`) are diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md index 0f583f726..09a52d35c 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md @@ -74,7 +74,7 @@ Upload debug information files to Sentry | `` | Files or directories to scan for debug information files | **Flags:** -- `-t, --type ... - Only upload files of this type (repeatable): dsym, elf, pe, pdb, portablepdb, wasm, breakpad, sourcebundle, jvm` +- `-t, --type ... - Only upload files of this type (repeatable): dsym, elf, pe, pdb, portablepdb, wasm, breakpad, sourcebundle, jvm, dxil` - `--id ... - Only upload the object with this debug id (repeatable)` - `--require-all - Fail if any --id value was not found among scanned files` - `--no-debug - Do not upload files whose only feature is debug/symbol info` diff --git a/packages/cli/src/commands/debug-files/upload.ts b/packages/cli/src/commands/debug-files/upload.ts index cce819e91..e2e2dbcac 100644 --- a/packages/cli/src/commands/debug-files/upload.ts +++ b/packages/cli/src/commands/debug-files/upload.ts @@ -253,6 +253,8 @@ function buildDifList( difs.push({ name: basename(file.path), debugId: file.debugId, + ...(file.codeId ? { codeId: file.codeId } : {}), + ...(file.shaderGroup ? { shaderGroup: file.shaderGroup } : {}), content: file.content, }); @@ -533,13 +535,13 @@ export const uploadCommand = buildCommand({ fullDescription: "Scan files and directories for native debug information files and " + "upload them to Sentry using the chunk-upload protocol. Supports " + - "Mach-O/dSYM, ELF, PE/PDB, Portable PDB, WebAssembly, Breakpad, and " + - "source bundles. Directories are scanned recursively.\n\n" + + "Mach-O/dSYM, ELF, PE/PDB, Portable PDB, WebAssembly, Breakpad, source " + + "bundles, and DXIL shader binaries. Directories are scanned recursively.\n\n" + "Org/project are auto-detected from DSN, env vars, or config defaults.\n\n" + "Filters:\n" + " --type Only upload files of the given type (repeatable):\n" + " dsym, elf, pe, pdb, portablepdb, wasm, breakpad,\n" + - " sourcebundle, jvm\n" + + " sourcebundle, jvm, dxil\n" + " --id Only upload the object with the given debug id (repeatable)\n" + " --no-debug / --no-unwind / --no-sources Drop files whose only\n" + " useful feature is the named one\n" + @@ -621,7 +623,7 @@ export const uploadCommand = buildCommand({ parse: choiceParser(VALID_DIF_TYPES), brief: "Only upload files of this type (repeatable): dsym, elf, pe, pdb, " + - "portablepdb, wasm, breakpad, sourcebundle, jvm", + "portablepdb, wasm, breakpad, sourcebundle, jvm, dxil", optional: true, variadic: true, }, diff --git a/packages/cli/src/lib/api/debug-files.ts b/packages/cli/src/lib/api/debug-files.ts index a990b1f5b..ae8851371 100644 --- a/packages/cli/src/lib/api/debug-files.ts +++ b/packages/cli/src/lib/api/debug-files.ts @@ -52,6 +52,13 @@ export type DebugFileUpload = { * `skip_serializing_if none` in the wire format — omitted when absent. */ debugId?: string; + /** + * Code id for file types the server cannot read it from (a DXIL shader's + * Aftermath hash). Omitted from the wire format when absent. + */ + codeId?: string; + /** Shader group of a DXIL shader, stored with the file. Omitted when absent. */ + shaderGroup?: string; /** Pre-read raw file content (chunked as-is). */ content: Buffer; }; @@ -109,6 +116,22 @@ type DifAssembleResponse = InferOutput; // ── Internal types ────────────────────────────────────────────────── +/** + * Maximum number of files per assemble request. The server processes every + * file of a request before answering, and shader uploads bring tens of + * thousands of files, so one request for all of them ran for minutes. + */ +export const ASSEMBLE_BATCH_SIZE = 1000; + +/** One file's entry in the assemble request body. */ +type AssembleFileEntry = { + name: string; + debug_id?: string; + code_id?: string; + shader_group?: string; + chunks: string[]; +}; + /** Per-file chunk metadata computed before the assemble request. */ type ChunkedDif = { /** The file being uploaded. */ @@ -141,21 +164,31 @@ type AssembleEvaluation = { */ function buildAssembleBody( chunked: ChunkedDif[], -): Record { - const body: Record< - string, - { name: string; debug_id?: string; chunks: string[] } - > = {}; +): Record { + const body: Record = {}; for (const cd of chunked) { body[cd.overallChecksum] = { name: cd.dif.name, ...(cd.dif.debugId ? { debug_id: cd.dif.debugId } : {}), + ...(cd.dif.codeId ? { code_id: cd.dif.codeId } : {}), + ...(cd.dif.shaderGroup ? { shader_group: cd.dif.shaderGroup } : {}), chunks: cd.chunks.map((c) => c.sha1), }; } return body; } +/** Split the files into assemble bodies of at most {@link ASSEMBLE_BATCH_SIZE}. */ +function buildAssembleBodies( + chunked: ChunkedDif[], +): Record[] { + const bodies: Record[] = []; + for (let i = 0; i < chunked.length; i += ASSEMBLE_BATCH_SIZE) { + bodies.push(buildAssembleBody(chunked.slice(i, i + ASSEMBLE_BATCH_SIZE))); + } + return bodies; +} + /** * Evaluate an assemble response for completion and outstanding chunks. * @@ -266,22 +299,26 @@ async function uploadMissing(params: { } } -/** POST the assemble request and parse the keyed response. */ +/** POST one assemble request per batch and merge the keyed responses. */ async function postAssemble( regionUrl: string, endpoint: string, - body: unknown, + bodies: readonly unknown[], ): Promise { - const { data } = await apiRequestToRegion( - regionUrl, - endpoint, - { - method: "POST", - body, - schema: DifAssembleResponseSchema, - }, - ); - return data; + const merged: DifAssembleResponse = {}; + for (const body of bodies) { + const { data } = await apiRequestToRegion( + regionUrl, + endpoint, + { + method: "POST", + body, + schema: DifAssembleResponseSchema, + }, + ); + Object.assign(merged, data); + } + return merged; } /** Build per-file results from the last-observed assemble response. */ @@ -444,7 +481,7 @@ export async function uploadDebugFiles( const regionUrl = await resolveOrgRegion(org); const endpoint = `projects/${org}/${project}/files/difs/assemble/`; - const body = buildAssembleBody(chunked); + const bodies = buildAssembleBodies(chunked); // Clamp the caller's requested wait to the server's advertised maximum. // Only in wait mode — in no-wait mode `maxWaitMs` is purely a hang guard for @@ -453,7 +490,7 @@ export async function uploadDebugFiles( ? clampMaxWait(maxWaitMs, serverOptions.maxWait ?? 0) : maxWaitMs; const deadline = Date.now() + effectiveMaxWaitMs; - let response = await postAssemble(regionUrl, endpoint, body); + let response = await postAssemble(regionUrl, endpoint, bodies); let evaluation = evaluateAssembly(response, chunked, wait); await uploadMissing({ chunked, @@ -482,7 +519,7 @@ export async function uploadDebugFiles( } await new Promise((r) => setTimeout(r, ASSEMBLE_POLL_INTERVAL_MS)); - response = await postAssemble(regionUrl, endpoint, body); + response = await postAssemble(regionUrl, endpoint, bodies); evaluation = evaluateAssembly(response, chunked, wait); await uploadMissing({ chunked, diff --git a/packages/cli/src/lib/dif/scan.ts b/packages/cli/src/lib/dif/scan.ts index d5317ab4e..492bde995 100644 --- a/packages/cli/src/lib/dif/scan.ts +++ b/packages/cli/src/lib/dif/scan.ts @@ -19,7 +19,7 @@ */ import { open, readFile, stat } from "node:fs/promises"; -import { extname, resolve } from "node:path"; +import { basename, extname, resolve } from "node:path"; import { ValidationError } from "../errors.js"; import { logger } from "../logger.js"; import { type WalkEntry, walkFiles } from "../scan/index.js"; @@ -30,6 +30,7 @@ import { peekFormat, splitDebugFile, } from "./index.js"; +import { type ShaderInfo, ShaderManifestCache } from "./shader-manifest.js"; import { DEFAULT_MAX_ZIP_TOTAL_SIZE, readZipDifEntries } from "./zip.js"; const log = logger.withTag("dif.scan"); @@ -56,8 +57,15 @@ const TYPE_TO_FORMAT: Readonly> = { breakpad: "breakpad", sourcebundle: "sourcebundle", jvm: "sourcebundle", + dxil: "dxil", }; +/** Header every DXIL container (a compiled shader written by DXC) starts with. */ +const DXIL_HEADER = Buffer.from("DXBC", "ascii"); + +/** A DXIL file is named after its 32-hex compiler hash, which is its debug id. */ +const DXIL_STEM_RE = /^[0-9a-f]{32}$/i; + /** Accepted `--type` filter values. */ export const VALID_DIF_TYPES: readonly string[] = Object.keys(TYPE_TO_FORMAT); @@ -108,6 +116,10 @@ export type PreparedDif = { debugId?: string; /** The object in `content` that passed the filters (for reporting). */ objects: DifObjectInfo[]; + /** Code id the server cannot derive from the file (DXIL: the Aftermath hash). */ + codeId?: string; + /** Shader group of a DXIL file, from the manifest next to it. */ + shaderGroup?: string; }; /** @@ -397,7 +409,7 @@ async function peekHeader(path: string): Promise { if (header.length === 0) { return null; } - const format = peekFormat(header); + const format = detectFormat(header); if (format === "unknown") { return null; } @@ -475,6 +487,7 @@ export async function prepareDifs( const maxFileSize = options.maxFileSize ?? 0; const scanZips = options.scanZips ?? true; const maxZipTotalSize = options.maxZipTotalSize ?? DEFAULT_MAX_ZIP_TOTAL_SIZE; + const manifests = new ShaderManifestCache(); let oversizedCount = 0; for (const path of paths) { @@ -506,6 +519,7 @@ export async function prepareDifs( path, filters, maxFileSize, + manifests, ); if (oversized) { oversizedCount += 1; @@ -536,6 +550,7 @@ async function prepareFileDif( path: string, filters: DifFilters, maxFileSize: number, + manifests: ShaderManifestCache, ): Promise<{ difs: PreparedDif[]; oversized: boolean }> { const peeked = await peekHeader(path); if (!peeked) { @@ -567,9 +582,12 @@ async function prepareFileDif( ); return { difs: [], oversized: false }; } + // A shader's code id and group come from the manifest next to it. + const shaderInfo = + peeked.format === "dxil" ? await manifests.lookup(path) : undefined; // readMatchedDif reports an oversized embedded PPDB (of a requested type) so // it too drives the exit code, matching the on-disk size gate above. - return await readMatchedDif(path, filters, maxFileSize); + return await readMatchedDif(path, filters, maxFileSize, shaderInfo); } /** @@ -690,9 +708,85 @@ function embeddedPpdbDif( }; } +/** + * Detect a container format from leading header bytes: DXIL containers, which + * the object parser does not know, are recognised here; everything else is + * delegated to {@link peekFormat}. + */ +function detectFormat(header: Uint8Array): string { + return isDxil(header) ? "dxil" : peekFormat(header); +} + +/** Whether the bytes start with the DXIL container header. */ +function isDxil(bytes: Uint8Array): boolean { + return ( + bytes.length >= DXIL_HEADER.length && + DXIL_HEADER.equals(bytes.subarray(0, DXIL_HEADER.length)) + ); +} + /** Detect the format of an in-memory buffer from its leading header bytes. */ function peekBufferFormat(content: Buffer): string { - return peekFormat(new Uint8Array(content.subarray(0, PEEK_HEADER_BYTES))); + return detectFormat(new Uint8Array(content.subarray(0, PEEK_HEADER_BYTES))); +} + +/** The hyphenated debug id of a DXIL file named after its 32-hex compiler hash. */ +function dxilDebugId(hex: string): string { + const h = hex.toLowerCase(); + return `${h.slice(0, 8)}-${h.slice(8, 12)}-${h.slice(12, 16)}-${h.slice(16, 20)}-${h.slice(20)}`; +} + +/** + * Prepare a DXIL shader binary. Its debug id is the compiler hash the engine + * uses as the file name; the code id and shader group come from the manifest + * row, when one was found. Shaders carry none of the symbol/debug/unwind/source + * features, so only the format and `--id` filters apply. + */ +function dxilDif( + displayPath: string, + content: Buffer, + filters: DifFilters, + info?: ShaderInfo, +): PreparedDif[] { + const stem = basename(displayPath, extname(displayPath)); + if (!DXIL_STEM_RE.test(stem)) { + log.warn( + `Skipping ${displayPath}: a DXIL file must be named after its 32-hex shader hash`, + ); + return []; + } + const debugId = dxilDebugId(stem); + // `--id` may be given as the bare file stem or in the hyphenated form. + if ( + !( + formatMatches("dxil", filters.formats) && + (idMatches(debugId, filters.ids) || idMatches(stem, filters.ids)) + ) + ) { + return []; + } + return [ + { + path: displayPath, + content, + debugId, + objects: [ + { + debugId, + codeId: info?.codeId ?? null, + arch: "any", + fileFormat: "dxil", + kind: "shader", + hasSymbols: false, + hasDebugInfo: false, + hasUnwindInfo: false, + hasSources: false, + }, + ], + ...(info?.codeId ? { codeId: info.codeId } : {}), + ...(info?.shaderGroup ? { shaderGroup: info.shaderGroup } : {}), + }, + ]; } /** @@ -715,7 +809,14 @@ function difFromBuffer( content: Buffer, filters: DifFilters, maxFileSize: number, + shaderInfo?: ShaderInfo, ): { difs: PreparedDif[]; oversized: boolean } { + if (isDxil(content)) { + return { + difs: dxilDif(displayPath, content, filters, shaderInfo), + oversized: false, + }; + } const ppdb = embeddedPpdbDif(displayPath, content, filters, maxFileSize); return { difs: [...matchedDifs(displayPath, content, filters), ...ppdb.difs], @@ -739,6 +840,7 @@ async function readMatchedDif( path: string, filters: DifFilters, maxFileSize: number, + shaderInfo?: ShaderInfo, ): Promise<{ difs: PreparedDif[]; oversized: boolean }> { let content: Buffer; try { @@ -747,7 +849,7 @@ async function readMatchedDif( log.debug(`Skipping unreadable file: ${path}`, err); return { difs: [], oversized: false }; } - return difFromBuffer(path, content, filters, maxFileSize); + return difFromBuffer(path, content, filters, maxFileSize, shaderInfo); } /** diff --git a/packages/cli/src/lib/dif/shader-manifest.ts b/packages/cli/src/lib/dif/shader-manifest.ts new file mode 100644 index 000000000..ffb51936e --- /dev/null +++ b/packages/cli/src/lib/dif/shader-manifest.ts @@ -0,0 +1,99 @@ +/** + * Shader manifest lookup for DXIL shader binaries. + * + * An Unreal cook exports one `.dxil` file per compiled shader. The GPU crash + * decoder identifies a shader by NVIDIA's own hash of its bytes, which only + * NVIDIA's Windows library can compute, so the Sentry Unreal plugin computes it + * at cook time and writes `sentry-shader-manifest.tsv` next to the files. Each + * row is `\t\t[\t]`; the + * hash is sent as the file's `code_id` and the group as `shader_group`. + */ + +import { readFile } from "node:fs/promises"; +import { basename, dirname, join } from "node:path"; +import { logger } from "../logger.js"; + +const log = logger.withTag("dif.shader-manifest"); + +/** File name of the manifest the Unreal plugin writes next to the shaders. */ +export const SHADER_MANIFEST_FILE_NAME = "sentry-shader-manifest.tsv"; + +/** Largest value of a 64-bit shader hash. */ +const MAX_U64 = 0xffffffffffffffffn; + +/** Manifest metadata of a single `.dxil` file. */ +export type ShaderInfo = { + /** NVIDIA Aftermath shader hash as 16 lowercase hex digits. */ + codeId: string; + /** Engine shader group, e.g. `Global/FHeavyComputeLoop/0`, when recorded. */ + shaderGroup?: string; +}; + +/** `.dxil` file names mapped to their manifest metadata. */ +export type ShaderManifest = ReadonlyMap; + +/** + * Parse manifest contents. Rows with a missing or non-numeric hash or a missing + * path are skipped; a blank group column means no group. + */ +export function parseShaderManifest(contents: string): ShaderManifest { + const manifest = new Map(); + for (const line of contents.split("\n")) { + const [hashColumn, , pathColumn, groupColumn] = line.split("\t"); + if (!(hashColumn && pathColumn)) { + continue; + } + const hashText = hashColumn.trim(); + if (!/^\d+$/.test(hashText)) { + continue; + } + const hash = BigInt(hashText); + if (hash > MAX_U64) { + continue; + } + const fileName = pathColumn.trim().split(/[\\/]/).pop(); + if (!fileName) { + continue; + } + const shaderGroup = groupColumn?.trim(); + manifest.set(fileName, { + codeId: hash.toString(16).padStart(16, "0"), + ...(shaderGroup ? { shaderGroup } : {}), + }); + } + return manifest; +} + +/** + * Load the manifest written next to the shader files in `dir`. A missing or + * unreadable manifest yields an empty map: the binaries are then uploaded + * without a code id, which is still a valid debug file. + */ +export async function loadShaderManifest(dir: string): Promise { + const path = join(dir, SHADER_MANIFEST_FILE_NAME); + try { + return parseShaderManifest(await readFile(path, "utf8")); + } catch (err) { + log.debug( + `No shader manifest at ${path}; DXIL files are uploaded without code id`, + err, + ); + return new Map(); + } +} + +/** Per-directory manifest cache for one scan. */ +export class ShaderManifestCache { + private readonly manifests = new Map>(); + + /** The manifest entry for the `.dxil` file at `path`, if any. */ + async lookup(path: string): Promise { + const dir = dirname(path); + let manifest = this.manifests.get(dir); + if (!manifest) { + manifest = loadShaderManifest(dir); + this.manifests.set(dir, manifest); + } + return (await manifest).get(basename(path)); + } +} diff --git a/packages/cli/test/fixtures/dif/shader/a223507a29b50f7132d44901c2228311.dxil b/packages/cli/test/fixtures/dif/shader/a223507a29b50f7132d44901c2228311.dxil new file mode 100644 index 0000000000000000000000000000000000000000..5b098815330e4e0c9ed2d474d7f748ce965cb5eb GIT binary patch literal 7672 zcmchcd0bOh+Q)D9g%A=J39{XQ21LXpD2r&C1rQK1EK+K%69^!sh(RgilG1FDpddl9 zMe7o(;D$C@rQkT71P~&_t7!Z7Wt1|Yh-j%3X{Up2U;EyBlN;>3pZDMQd_L#ibDn#C z=bYzzo^$REB<6`F`pYeeA1+$Fa_nROJ@-HRX#oZRfVn^bAVYT{l%-IXL0JLi)zCu) zlmx3h3o^{vR;UW&)#;t`5!2soK z466?=dH}c-07!*2Fkdf?Obn)zYbh8EPzM6QcF4vLF;Kw58oXENX&rQVq-!;Ld#2ad zyXpGU8ADc@5K9EKWGfdd)u;{eSZ=sBpH(@;1e-^rH^0)Y(M{@w(UEv+^A%cu0Sj6z zp*YOb}U?)>Qg?gR+P<#rgb)gDud<$ zrQwa_UM_9$z{02xdxFHX-3<}B1+z^Ll6Cp4WOj_4XGkdR`jxVmeCtWf`MkPY%ggFwS3v37o^Q=F~ycqJ-19)kD3@hb01wB;I6cNu~9x1AFp=rmXHI ztpRp*&@)lcpopHL_Lyl6=vN1giGt=V0V5NEJ?fwlfHrOMuvk2d(xa8Pbq%-#UGM54 zt-q~j*|0c%9Jl6c?doQ2fDwz1tPM5+rb?CwsAUo#o&WItV|8m(YC{%?Wfg>kP<#OM zN0HZJQ?nB(hwfm9K4&l!FkWh`c1(nqwS+<;2zYt#u;?pg`j`^^?h+hScK=LD-!@F& z16=`+JzUZb6KN+%+GeCzpoLv%p+ZK28w?YlzSgWtNZKPKRSpxU7^EskOBo&N!O?OG zdSoI&ZMKcmp?;h_AeL1gi;V2hq(qV!28Zt;$$y{DBg#rf^=+w>Nu)ssrS z&p2kqi|A%|t)EG>u#7b*biXI-H51=65|w(@$)(*Zu3tIF>%PMOVo8_z@|7i5xmUhO z08JfWN5rRB-_F`5WEoV`KK$q>#_0RB2Shd2%a{bHnp2h|IXgR z0!^=*OdLE(2??U}ysQd#};^>lI_KAA`+F9~0A7RtY~bBN^d*G*^Z@Q{g| zvW|3m^5TJv^JSL~G?d>2Nac3o%>T3F%L0uDzgV^E+?DQCF{?T{e6#^wq5x?yw))cK zz3IXKweTv{6^;44;oj7E&vgH=sb{k1{^0cB3RNnj=Ah;% zlz(S0P^o@rI+`MbOscDu!nCnx)5d$#=R0y~mE**n=6n?{-lUBvE34WT^=@HNd1bc7 zMD;td|D7YFo9}<}pjlNSu6?+3a97lh zqMh5{-ytvBnT@?f_dBufi2w1;L0`Xp)c@PjW*Kx)!22){+K16(RY2cFK(RMwjX@Hp z>#Y~7NV|8DcD7W|rT~xc8T21j^k|wmsK*+3yEUMgOF)`w{fBVS+hI*TZq3lH`bq_9 zFPXGEMXykh_9$>wyYvcB?>EGx&lu?wT#s_D$0Ix4E~Q5^`U1z;0V8bq?NH3EOSqv; z7?#9e#jU=fU9H0<4B5S_WTd?%q}USu4(O`VD~x)Dj8qj)s+uD}-TqOf{{vZH!P4(& z(eG|iM?SaHpO4f3X`#<>aRs*4fXUW?A>=xmA27%c=u-y{<`cg$LT@oid<$~z>Wyob z#2Ii2&ArQ^L4VaGe2q&m_tN^aMQ$Ui+QmJ0v-1eX+29}70 zJ_P1rI=5MeA;XPVrXYD1a zT2{Mddiq?RySdF@jaa-Y*5@Re4|y@GD;~rc-9P@v>Q|LehX(D4s!6g&u;W3lody{N z|B1}j9mt3LZiYT=z3`$)`0WFs0GgZEGMlYrA;p+ki>Z0D@^NliTW(2FZsFow+2f6y z+cxGEZA^JAjVXF_M>KHr%^R4<*=aYkyo)jwkNu_9goRRKo0Qd-8Spq0T%5J2C@Z`z zD~gcKPs)yK%U(^$5hvv&-^|G%Y|KhZvaqb1E?oPyfjLrovH$NZ^~G;We`#QLT-$Wv z@5qxmr1ISuOH{2bH4x=^kgO?T)iYHf`O`5LhO7Y>(&hMwEho>%CIcGK@LLaJ{M8L< z35Fvr4~}z@hZ-yL@aG0t-t%{w1*yJjfLsPbi4;I11Bp+@NuNvUkB8k*!?c}rO-7#@ zcNv}n)GrkDCw7 zp2Y&l8mro4uLWvOf|}c{y-U2$agQ?a&iXA$L_jN42ToDM+@oCbDM}x=KmqpMs9BGcA=oT>sd%3()kFTka~U@!M#VXcr*%g?+P}SI^22L_@IH+gE$f@NzXYqdTa^2Hfb$E@lK89P4b<4pD za$E4qjyMVw7PQ4|~%l5Z@1scMMh>6eEeRtP7VnmAy~|+rp9F=)XCICL6l?~ z9ohq?Rfk^1VK40e39wRYK8RFDj5NH;&iGI z!;KuCl)uG5SM_<}dJR(xFYwuCg-Hi2$&xpOX3fNg@kJ(pwLqs<(r6kqcpioA@T_MA zK*x{hW*9%jUww{v9O_qlZWQTPhH#M6=fH`*8ddD;c8X{%)vsRl)uk#MeSShhh=@^Z~&jri$C-O zc_uc4fM3e0s@aI8s`5p_wt+biS`6g;jJy6Y!s=X?u7{k>vMCV<%(H?~pLVZ_C?g$Uv`wt;E33o&2LM-g-7 zQJZCghY=8BQ}j|V81bA)(COhqeCx-kL{T`J96yt}L&0o7p%EBhJ-tM-abl8yE?3 zAwK^o7DasaQS@e6=j?JgiW5#Dh^NJZ7+)9Sy5sjy#CwiknZSPMa|K2`nt2?FV&7Tb zpXU)jH)o)Tzc;)4vbxPQ2(fCz41)Nsv9TuEg}8ca6^eMz*77dPH%Y}XVx=MsMtr(f zNn7bcEEily5hn@u^b$DB<8Tn-=RYJMh+`(@pUorwiE z7wq8A8(fHoJ$^IN9G-Th_W_Ibn~j)kF!Kir~sn<@N;2q8{3 zR3nIY+Xif*F2o^)Z=i_5!i3GTD6d#Jip9Smh({!X&L9`!)Yexh;#I9<6W9w!{sbdt zdA1@^96ZZ=IggmuBt;RsHF@~5?lgNqh{vT=0(?;Xx3Tf@S{LH-k{T3oX~_p&mi}TT zj94!VM~-8Il9uQ~{7Az0Mx$6V3q^6v1sHL0r368oG${}BbVV_LuoXqj8w`#zchv<# zh)Y6)sqjIuJS2E$lMAt#$s#yDC8g(>+a`M3m;ON4fY0Kra0GFDQ{&B4 z7ve4NHlv6)y<6X9sd`cmBmPLb1UZZAl(Y>l#PW6LQN&5>O8W>L!FD`^_{$&D5X3o? z@()SA5om*w`~p72?m zBR~+p-_+Q)&V@Lm^dyQnt@PtAOY!tE7;&300y&HKD`{I?h#zHz_~Zd=?S76Q2UaNae*VChKx81jTZ)R3>U(hE&kt>vebc6dschLI^vLQ~r6nv(p@QlC6an#<=) zxl|g~zpWXj{};^#m_8$BmS=X+Kk$cJ8)cYy>sIGy&zDD+aLHwd`>Bu<8E&5;PTaQ* z49`2!-u9-N@V(24wD-U~m2guiyfoC za>@@n<$ra`i=CrKJAMDe>HB(ze2n9eS9v)m|D{v@)G2@8IeLQA_v23AH#p?C!yNMG zPWjJH`EO3S&N;f+>3gfwcd0{O9qy2C^>&ONOGVJ#3?v}rA2vGV6=@Fnt~Q5!Gz*r) We@j7=G%Gb1{+kl~e_GTA-~R`A{$Lsa literal 0 HcmV?d00001 diff --git a/packages/cli/test/fixtures/dif/shader/sentry-shader-manifest.tsv b/packages/cli/test/fixtures/dif/shader/sentry-shader-manifest.tsv new file mode 100644 index 000000000..4c4ab8e41 --- /dev/null +++ b/packages/cli/test/fixtures/dif/shader/sentry-shader-manifest.tsv @@ -0,0 +1,2 @@ +15683173249811795820 a223507a29b50f7132d44901c2228311.pdb D:/Build/Saved/ShaderSymbols/PCD3D_SM5/a223507a29b50f7132d44901c2228311.dxil Global/FSentryTestCS/0 +1 missing.pdb missing.dxil diff --git a/packages/cli/test/lib/api/debug-files.test.ts b/packages/cli/test/lib/api/debug-files.test.ts index 70fa672f4..05576eb3d 100644 --- a/packages/cli/test/lib/api/debug-files.test.ts +++ b/packages/cli/test/lib/api/debug-files.test.ts @@ -10,6 +10,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { getChunkUploadOptions } from "../../../src/lib/api/chunk-upload.js"; import { + ASSEMBLE_BATCH_SIZE, type DebugFileUpload, uploadDebugFiles, } from "../../../src/lib/api/debug-files.js"; @@ -312,6 +313,86 @@ describe("uploadDebugFiles", () => { expect(results).toHaveLength(2); }); + test("includes code_id and shader_group only when provided", async () => { + apiRequestToRegionMock.mockImplementation( + async (_url: string, _endpoint: string, init: { body: object }) => { + const body = init.body as Record; + const data: Record = {}; + for (const key of Object.keys(body)) { + data[key] = { state: "ok" }; + } + return { data }; + }, + ); + + await uploadDebugFiles({ + ...SOLE, + difs: [ + { + ...makeDif( + "shader.dxil", + "DXBC shader", + "a223507a-29b5-0f71-32d4-4901c2228311", + ), + codeId: "d9a5d2fa49f0cf6c", + shaderGroup: "Global/FSentryTestCS/0", + }, + ], + wait: false, + maxWaitMs: 1000, + }); + const [entry] = Object.values(lastAssembleBody()) as Record< + string, + unknown + >[]; + expect(entry?.code_id).toBe("d9a5d2fa49f0cf6c"); + expect(entry?.shader_group).toBe("Global/FSentryTestCS/0"); + + await uploadDebugFiles({ + ...SOLE, + difs: [makeDif("libfoo.so", "ELF debug bytes", "abc123-def")], + wait: false, + maxWaitMs: 1000, + }); + const [plain] = Object.values(lastAssembleBody()) as Record< + string, + unknown + >[]; + expect(plain).not.toHaveProperty("code_id"); + expect(plain).not.toHaveProperty("shader_group"); + }); + + test("splits the assemble request into batches and merges the responses", async () => { + apiRequestToRegionMock.mockImplementation( + async (_url: string, _endpoint: string, init: { body: object }) => { + const body = init.body as Record; + const data: Record = {}; + for (const key of Object.keys(body)) { + data[key] = { state: "ok" }; + } + return { data }; + }, + ); + const difs = Array.from({ length: ASSEMBLE_BATCH_SIZE + 1 }, (_, i) => + makeDif(`f${i}.so`, `bytes ${i}`, `id-${i}`), + ); + + const results = await uploadDebugFiles({ + ...SOLE, + difs, + wait: false, + maxWaitMs: 1000, + }); + + expect(apiRequestToRegionMock).toHaveBeenCalledTimes(2); + const sizes = apiRequestToRegionMock.mock.calls.map( + (call) => Object.keys(call[2].body as object).length, + ); + expect(sizes).toEqual([ASSEMBLE_BATCH_SIZE, 1]); + expect(results).toHaveLength(ASSEMBLE_BATCH_SIZE + 1); + expect(results.every((r) => r.state === "ok")).toBe(true); + }); + test("re-sends every chunk when server response omits a file's entry", async () => { // First call: server has no record of either file (entries missing). // Second call: server holds both files. No-wait completes on the second diff --git a/packages/cli/test/lib/dif/dxil.test.ts b/packages/cli/test/lib/dif/dxil.test.ts new file mode 100644 index 000000000..3d8c11902 --- /dev/null +++ b/packages/cli/test/lib/dif/dxil.test.ts @@ -0,0 +1,149 @@ +/** + * Unit tests for DXIL shader binaries in the DIF scanner: detection by the + * container header, the debug id taken from the file name, the manifest + * lookup, and how the `--type`/`--id` filters apply. + */ + +import { readFileSync } from "node:fs"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { + buildDifFilters, + prepareDifs, + scanPaths, +} from "../../../src/lib/dif/scan.js"; +import { SHADER_MANIFEST_FILE_NAME } from "../../../src/lib/dif/shader-manifest.js"; + +/** A compiled shader exported by an Unreal Engine 5.8 cook. */ +const DXIL_NAME = "a223507a29b50f7132d44901c2228311.dxil"; +const DXIL_DEBUG_ID = "a223507a-29b5-0f71-32d4-4901c2228311"; +const DXIL_CODE_ID = "d9a5d2fa49f0cf6c"; +const DXIL_GROUP = "Global/FSentryTestCS/0"; + +const FIXTURE_DIR = new URL("../../fixtures/dif/shader/", import.meta.url); + +let tempDir: string; + +beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "df-dxil-test-")); +}); + +afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); +}); + +/** Copy the DXIL fixture into `tempDir` under `name`, with or without the manifest. */ +async function writeShaderFixture( + name: string, + options: { manifest: boolean }, +): Promise { + const path = join(tempDir, name); + await writeFile(path, readFileSync(new URL(DXIL_NAME, FIXTURE_DIR))); + if (options.manifest) { + await writeFile( + join(tempDir, SHADER_MANIFEST_FILE_NAME), + readFileSync(new URL(SHADER_MANIFEST_FILE_NAME, FIXTURE_DIR)), + ); + } + return path; +} + +describe("prepareDifs with DXIL shader binaries", () => { + test("recognises a shader by its header and names it from the file stem", async () => { + const path = await writeShaderFixture(DXIL_NAME, { manifest: true }); + const { prepared } = await prepareDifs( + await scanPaths([path]), + buildDifFilters({}), + ); + + expect(prepared).toHaveLength(1); + const dif = prepared[0]; + expect(dif?.debugId).toBe(DXIL_DEBUG_ID); + expect(dif?.codeId).toBe(DXIL_CODE_ID); + expect(dif?.shaderGroup).toBe(DXIL_GROUP); + expect(dif?.objects[0]?.fileFormat).toBe("dxil"); + expect(dif?.content.length).toBeGreaterThan(0); + }); + + test("uploads a shader without a manifest, just without code id", async () => { + const path = await writeShaderFixture(DXIL_NAME, { manifest: false }); + const { prepared } = await prepareDifs( + await scanPaths([path]), + buildDifFilters({}), + ); + + expect(prepared).toHaveLength(1); + expect(prepared[0]?.debugId).toBe(DXIL_DEBUG_ID); + expect(prepared[0]?.codeId).toBeUndefined(); + expect(prepared[0]?.shaderGroup).toBeUndefined(); + }); + + test("skips a shader whose name is not a 32-hex hash", async () => { + const path = await writeShaderFixture("renamed.dxil", { manifest: false }); + const { prepared } = await prepareDifs( + await scanPaths([path]), + buildDifFilters({}), + ); + + expect(prepared).toHaveLength(0); + }); + + test("honours --type and --id", async () => { + const path = await writeShaderFixture(DXIL_NAME, { manifest: true }); + const files = await scanPaths([path]); + + const byType = await prepareDifs( + files, + buildDifFilters({ types: ["dxil"] }), + ); + expect(byType.prepared).toHaveLength(1); + + const otherType = await prepareDifs( + files, + buildDifFilters({ types: ["dsym"] }), + ); + expect(otherType.prepared).toHaveLength(0); + + const byId = await prepareDifs( + files, + buildDifFilters({ ids: [DXIL_DEBUG_ID.toUpperCase()] }), + ); + expect(byId.prepared).toHaveLength(1); + + const byStem = await prepareDifs( + files, + buildDifFilters({ ids: [DXIL_NAME.replace(/\.dxil$/, "")] }), + ); + expect(byStem.prepared).toHaveLength(1); + + const otherId = await prepareDifs( + files, + buildDifFilters({ ids: ["00000000-0000-0000-0000-000000000001"] }), + ); + expect(otherId.prepared).toHaveLength(0); + }); + + test("is not dropped by the feature filters shaders cannot satisfy", async () => { + const path = await writeShaderFixture(DXIL_NAME, { manifest: false }); + const { prepared } = await prepareDifs( + await scanPaths([path]), + buildDifFilters({ noDebug: true, noUnwind: true, noSources: true }), + ); + + expect(prepared).toHaveLength(1); + }); + + test("counts an oversized shader like any other requested file", async () => { + const path = await writeShaderFixture(DXIL_NAME, { manifest: false }); + const { prepared, oversizedCount } = await prepareDifs( + await scanPaths([path]), + buildDifFilters({ types: ["dxil"] }), + { maxFileSize: 1 }, + ); + + expect(prepared).toHaveLength(0); + expect(oversizedCount).toBe(1); + }); +}); diff --git a/packages/cli/test/lib/dif/shader-manifest.test.ts b/packages/cli/test/lib/dif/shader-manifest.test.ts new file mode 100644 index 000000000..acfe081c9 --- /dev/null +++ b/packages/cli/test/lib/dif/shader-manifest.test.ts @@ -0,0 +1,93 @@ +/** + * Unit tests for the DXIL shader manifest: row parsing and the per-directory + * lookup that attaches a code id and shader group to a `.dxil` file. + */ + +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { + loadShaderManifest, + parseShaderManifest, + SHADER_MANIFEST_FILE_NAME, + ShaderManifestCache, +} from "../../../src/lib/dif/shader-manifest.js"; + +const MANIFEST = [ + "709220027517385083\t000057e2ed5c597bf3ca2651c6e13a1d.pdb\tD:\\x\\000057e2ed5c597bf3ca2651c6e13a1d.dxil\r", + "2\tb.pdb\tD:/y/b.dxil\tGlobalShaders", + "3\tc.pdb\tc.dxil\t Padded Group \r", + "4\td.pdb\td.dxil\t ", + "123\t", + "notanumber\tfoo.pdb\tfoo.dxil", + "99999999999999999999999\tbig.pdb\tbig.dxil", + "", +].join("\n"); + +let tempDir: string; + +beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "df-shader-manifest-")); +}); + +afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); +}); + +describe("parseShaderManifest", () => { + test("keys rows by file name with the hash as 16 hex digits", () => { + const manifest = parseShaderManifest(MANIFEST); + + expect(manifest.get("000057e2ed5c597bf3ca2651c6e13a1d.dxil")).toEqual({ + codeId: "09d7a7db7fc4397b", + }); + expect(manifest.get("b.dxil")).toEqual({ + codeId: "0000000000000002", + shaderGroup: "GlobalShaders", + }); + }); + + test("trims the group and drops a blank one", () => { + const manifest = parseShaderManifest(MANIFEST); + + expect(manifest.get("c.dxil")?.shaderGroup).toBe("Padded Group"); + expect(manifest.get("d.dxil")).toEqual({ codeId: "0000000000000004" }); + }); + + test("skips rows without a path, a numeric hash, or a 64-bit hash", () => { + const manifest = parseShaderManifest(MANIFEST); + + expect(manifest.size).toBe(4); + expect(manifest.has("foo.dxil")).toBe(false); + expect(manifest.has("big.dxil")).toBe(false); + }); +}); + +describe("loadShaderManifest", () => { + test("returns an empty manifest when the file is missing", async () => { + expect((await loadShaderManifest(tempDir)).size).toBe(0); + }); + + test("reads the manifest next to the shaders", async () => { + await writeFile(join(tempDir, SHADER_MANIFEST_FILE_NAME), MANIFEST); + + expect((await loadShaderManifest(tempDir)).get("b.dxil")?.codeId).toBe( + "0000000000000002", + ); + }); +}); + +describe("ShaderManifestCache", () => { + test("looks a file up in its own directory's manifest", async () => { + await writeFile(join(tempDir, SHADER_MANIFEST_FILE_NAME), MANIFEST); + const cache = new ShaderManifestCache(); + + expect(await cache.lookup(join(tempDir, "b.dxil"))).toEqual({ + codeId: "0000000000000002", + shaderGroup: "GlobalShaders", + }); + expect(await cache.lookup(join(tempDir, "unknown.dxil"))).toBeUndefined(); + expect(await cache.lookup(join(tempDir, "sub", "b.dxil"))).toBeUndefined(); + }); +}); From 8e6a441522624621bc02cca6a1f0c9bc75521887 Mon Sep 17 00:00:00 2001 From: Ivan Tustanivskyi Date: Wed, 4 Nov 2026 15:28:41 +0200 Subject: [PATCH 2/3] fix(cli): Batch chunk uploads and assemble again after delivering them Missing chunks were uploaded one request per chunk, so a debug file set of ten thousand small files took twenty minutes, and the processing deadline started before that upload. When the upload outlasted the deadline the command gave up without sending the assemble request that starts processing, reporting the files as not found or timed out although every chunk was on the server. Chunks are now packed into requests up to the server's chunksPerRequest and maxRequestSize limits, a chunk shared by several files is sent once, the assemble request is repeated right after the chunks are delivered, and the wait budget starts from that point. --- packages/cli/src/lib/api/chunk-upload.ts | 135 +++++++++++++----- packages/cli/src/lib/api/debug-files.ts | 58 +++++--- .../cli/test/lib/api/chunk-batches.test.ts | 108 ++++++++++++++ packages/cli/test/lib/api/debug-files.test.ts | 30 ++-- 4 files changed, 261 insertions(+), 70 deletions(-) create mode 100644 packages/cli/test/lib/api/chunk-batches.test.ts diff --git a/packages/cli/src/lib/api/chunk-upload.ts b/packages/cli/src/lib/api/chunk-upload.ts index 426069ed8..f10a1e80d 100644 --- a/packages/cli/src/lib/api/chunk-upload.ts +++ b/packages/cli/src/lib/api/chunk-upload.ts @@ -420,25 +420,67 @@ export async function uploadMissingChunks(params: { * Same wire format as {@link uploadChunk} but reads from a buffer * instead of a file handle. */ -async function uploadBufferChunk(params: { - chunk: ChunkInfo; - content: Buffer; +/** One chunk's bytes, keyed by its checksum, ready to upload. */ +export type ChunkPayload = { + sha1: string; + data: Buffer; +}; + +/** + * Group chunks into upload requests of at most `chunksPerRequest` chunks and + * `maxRequestSize` bytes (a chunk larger than the cap still travels alone). + * Debug file sets can hold tens of thousands of small files, so one request + * per chunk would take minutes; the server accepts repeated `file` fields. + */ +export function batchChunkPayloads( + chunks: readonly ChunkPayload[], + serverOptions: Pick< + ChunkServerOptions, + "chunksPerRequest" | "maxRequestSize" + >, +): ChunkPayload[][] { + const maxChunks = Math.max(1, serverOptions.chunksPerRequest); + const maxBytes = Math.max(1, serverOptions.maxRequestSize); + const batches: ChunkPayload[][] = []; + let batch: ChunkPayload[] = []; + let bytes = 0; + for (const chunk of chunks) { + if ( + batch.length > 0 && + (batch.length >= maxChunks || bytes + chunk.data.length > maxBytes) + ) { + batches.push(batch); + batch = []; + bytes = 0; + } + batch.push(chunk); + bytes += chunk.data.length; + } + if (batch.length > 0) { + batches.push(batch); + } + return batches; +} + +/** POST one batch of chunks as a multipart request, one field per chunk. */ +async function uploadChunkBatch(params: { + batch: readonly ChunkPayload[]; encoding: UploadEncoding | undefined; fetch: (url: string, init: RequestInit) => Promise; url: string; }): Promise { - const { chunk, content, encoding, fetch: authFetch, url } = params; - - const buf = content.subarray(chunk.offset, chunk.offset + chunk.size); - const payload = await encodeChunk(buf, encoding); + const { batch, encoding, fetch: authFetch, url } = params; const fieldName = encoding === "gzip" ? "file_gzip" : "file"; const form = new FormData(); - form.append( - fieldName, - new Blob([payload], { type: "application/octet-stream" }), - chunk.sha1, - ); + for (const chunk of batch) { + const payload = await encodeChunk(chunk.data, encoding); + form.append( + fieldName, + new Blob([payload], { type: "application/octet-stream" }), + chunk.sha1, + ); + } const init: RequestInit = { method: "POST", body: form }; if (encoding === "zstd") { @@ -460,31 +502,17 @@ async function uploadBufferChunk(params: { } /** - * Upload missing chunks from an in-memory buffer, in parallel. - * - * Same as {@link uploadMissingChunks} but reads chunk data from a buffer - * instead of a file. Used by DIF uploads (e.g. ProGuard) where raw bytes - * are chunked directly without wrapping in a ZIP. + * Upload chunks in batched requests (see {@link batchChunkPayloads}), with + * `concurrency` requests in flight at once. */ -export async function uploadMissingBufferChunks(params: { - chunks: ChunkInfo[]; - missingChecksums: Set; - content: Buffer; +export async function uploadChunkPayloads(params: { + chunks: readonly ChunkPayload[]; serverOptions: ChunkServerOptions; encoding: UploadEncoding | undefined; regionUrl: string; }): Promise { - const { - chunks, - missingChecksums, - content, - serverOptions, - encoding, - regionUrl, - } = params; - const missingChunks = chunks.filter((c) => missingChecksums.has(c.sha1)); - - if (missingChunks.length === 0) { + const { chunks, serverOptions, encoding, regionUrl } = params; + if (chunks.length === 0) { return; } @@ -494,11 +522,10 @@ export async function uploadMissingBufferChunks(params: { const { fetch: authFetch } = getSdkConfig(regionUrl, { idempotent: true }); await Promise.all( - missingChunks.map((chunk) => + batchChunkPayloads(chunks, serverOptions).map((batch) => limit(() => - uploadBufferChunk({ - chunk, - content, + uploadChunkBatch({ + batch, encoding, fetch: authFetch, url: serverOptions.url, @@ -508,6 +535,42 @@ export async function uploadMissingBufferChunks(params: { ); } +/** + * Upload missing chunks from an in-memory buffer, in parallel. + * + * Same as {@link uploadMissingChunks} but reads chunk data from a buffer + * instead of a file. Used by DIF uploads (e.g. ProGuard) where raw bytes + * are chunked directly without wrapping in a ZIP. + */ +export async function uploadMissingBufferChunks(params: { + chunks: ChunkInfo[]; + missingChecksums: Set; + content: Buffer; + serverOptions: ChunkServerOptions; + encoding: UploadEncoding | undefined; + regionUrl: string; +}): Promise { + const { + chunks, + missingChecksums, + content, + serverOptions, + encoding, + regionUrl, + } = params; + await uploadChunkPayloads({ + chunks: chunks + .filter((c) => missingChecksums.has(c.sha1)) + .map((c) => ({ + sha1: c.sha1, + data: content.subarray(c.offset, c.offset + c.size), + })), + serverOptions, + encoding, + regionUrl, + }); +} + /** * Poll an assemble endpoint until the server reports completion. * diff --git a/packages/cli/src/lib/api/debug-files.ts b/packages/cli/src/lib/api/debug-files.ts index ae8851371..c40e5387a 100644 --- a/packages/cli/src/lib/api/debug-files.ts +++ b/packages/cli/src/lib/api/debug-files.ts @@ -28,13 +28,14 @@ import { type AssembleResponse, AssembleResponseSchema, type ChunkInfo, + type ChunkPayload, type ChunkServerOptions, DEFAULT_MAX_DIF_SIZE, getChunkUploadOptions, hashBuffer, pickUploadEncoding, type UploadEncoding, - uploadMissingBufferChunks, + uploadChunkPayloads, } from "./chunk-upload.js"; import { apiRequestToRegion } from "./infrastructure.js"; @@ -274,7 +275,10 @@ function classifyEntry( return { held: true, terminal, missing: [] }; } -/** Upload any chunks the server reported missing, across all files. */ +/** + * Upload every chunk the server reported missing, across all files, in + * batched requests. A chunk shared by several files is sent once. + */ async function uploadMissing(params: { chunked: ChunkedDif[]; missingChecksums: Set; @@ -287,16 +291,26 @@ async function uploadMissing(params: { if (missingChecksums.size === 0) { return; } + const payloads: ChunkPayload[] = []; + const queued = new Set(); for (const cd of chunked) { - await uploadMissingBufferChunks({ - chunks: cd.chunks, - missingChecksums, - content: cd.dif.content, - serverOptions, - encoding, - regionUrl, - }); + for (const chunk of cd.chunks) { + if (!missingChecksums.has(chunk.sha1) || queued.has(chunk.sha1)) { + continue; + } + queued.add(chunk.sha1); + payloads.push({ + sha1: chunk.sha1, + data: cd.dif.content.subarray(chunk.offset, chunk.offset + chunk.size), + }); + } } + await uploadChunkPayloads({ + chunks: payloads, + serverOptions, + encoding, + regionUrl, + }); } /** POST one assemble request per batch and merge the keyed responses. */ @@ -489,17 +503,25 @@ export async function uploadDebugFiles( const effectiveMaxWaitMs = wait ? clampMaxWait(maxWaitMs, serverOptions.maxWait ?? 0) : maxWaitMs; - const deadline = Date.now() + effectiveMaxWaitMs; let response = await postAssemble(regionUrl, endpoint, bodies); let evaluation = evaluateAssembly(response, chunked, wait); - await uploadMissing({ - chunked, - missingChecksums: evaluation.missingChecksums, - serverOptions, - encoding, - regionUrl, - }); + if (evaluation.missingChecksums.size > 0) { + await uploadMissing({ + chunked, + missingChecksums: evaluation.missingChecksums, + serverOptions, + encoding, + regionUrl, + }); + // The server only starts assembling once it is asked again with the + // chunks in place, so re-assemble right after delivering them. + response = await postAssemble(regionUrl, endpoint, bodies); + evaluation = evaluateAssembly(response, chunked, wait); + } + // The wait budget covers server-side processing, not the chunk upload, + // which for large sets can itself take longer than the budget. + const deadline = Date.now() + effectiveMaxWaitMs; while (!evaluation.done) { if (Date.now() >= deadline) { if (wait) { diff --git a/packages/cli/test/lib/api/chunk-batches.test.ts b/packages/cli/test/lib/api/chunk-batches.test.ts new file mode 100644 index 000000000..6b8001957 --- /dev/null +++ b/packages/cli/test/lib/api/chunk-batches.test.ts @@ -0,0 +1,108 @@ +/** + * Unit tests for batched chunk uploads: how chunks are grouped into requests + * against the server's limits, and that one request carries a whole batch. + */ + +import { afterEach, describe, expect, test, vi } from "vitest"; + +const fetchMock = vi.hoisted(() => + vi.fn(async () => new Response(null, { status: 200 })), +); + +vi.mock("../../../src/lib/sentry-client.js", () => ({ + getSdkConfig: () => ({ fetch: fetchMock }), +})); + +import { + batchChunkPayloads, + type ChunkPayload, + uploadChunkPayloads, +} from "../../../src/lib/api/chunk-upload.js"; + +const SERVER_OPTIONS = { + url: "https://us.sentry.io/api/0/chunk-upload/", + chunkSize: 8192, + chunksPerRequest: 3, + maxRequestSize: 100, + hashAlgorithm: "sha1", + concurrency: 2, + compression: [] as string[], +}; + +function payload(sha1: string, size: number): ChunkPayload { + return { sha1, data: Buffer.alloc(size, 1) }; +} + +afterEach(() => { + fetchMock.mockClear(); +}); + +describe("batchChunkPayloads", () => { + test("fills a request up to chunksPerRequest", () => { + const chunks = ["a", "b", "c", "d", "e"].map((id) => payload(id, 10)); + const batches = batchChunkPayloads(chunks, SERVER_OPTIONS); + expect(batches.map((b) => b.map((c) => c.sha1))).toEqual([ + ["a", "b", "c"], + ["d", "e"], + ]); + }); + + test("starts a new request when maxRequestSize would be exceeded", () => { + const chunks = [payload("a", 60), payload("b", 50), payload("c", 40)]; + const batches = batchChunkPayloads(chunks, SERVER_OPTIONS); + expect(batches.map((b) => b.map((c) => c.sha1))).toEqual([ + ["a"], + ["b", "c"], + ]); + }); + + test("sends an oversized chunk on its own", () => { + const batches = batchChunkPayloads( + [payload("big", 500), payload("small", 1)], + SERVER_OPTIONS, + ); + expect(batches.map((b) => b.map((c) => c.sha1))).toEqual([ + ["big"], + ["small"], + ]); + }); + + test("returns nothing for no chunks", () => { + expect(batchChunkPayloads([], SERVER_OPTIONS)).toEqual([]); + }); +}); + +describe("uploadChunkPayloads", () => { + test("posts one multipart request per batch with one field per chunk", async () => { + const chunks = ["a", "b", "c", "d"].map((id) => payload(id, 10)); + + await uploadChunkPayloads({ + chunks, + serverOptions: SERVER_OPTIONS, + encoding: undefined, + regionUrl: "https://us.sentry.io", + }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const forms = fetchMock.mock.calls.map( + (call) => (call as unknown as [string, { body: FormData }])[1].body, + ); + expect( + forms.map((f) => f.getAll("file").length).sort((a, b) => a - b), + ).toEqual([1, 3]); + const names = forms.flatMap((f) => + f.getAll("file").map((entry) => (entry as File).name), + ); + expect(names.sort()).toEqual(["a", "b", "c", "d"]); + }); + + test("makes no request for no chunks", async () => { + await uploadChunkPayloads({ + chunks: [], + serverOptions: SERVER_OPTIONS, + encoding: undefined, + regionUrl: "https://us.sentry.io", + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/test/lib/api/debug-files.test.ts b/packages/cli/test/lib/api/debug-files.test.ts index 05576eb3d..acd57f8ef 100644 --- a/packages/cli/test/lib/api/debug-files.test.ts +++ b/packages/cli/test/lib/api/debug-files.test.ts @@ -16,12 +16,10 @@ import { } from "../../../src/lib/api/debug-files.js"; import { ApiError } from "../../../src/lib/errors.js"; -const { apiRequestToRegionMock, uploadMissingBufferChunksMock } = vi.hoisted( - () => ({ - apiRequestToRegionMock: vi.fn(), - uploadMissingBufferChunksMock: vi.fn(() => Promise.resolve()), - }), -); +const { apiRequestToRegionMock, uploadChunkPayloadsMock } = vi.hoisted(() => ({ + apiRequestToRegionMock: vi.fn(), + uploadChunkPayloadsMock: vi.fn(() => Promise.resolve()), +})); vi.mock("../../../src/lib/region.js", () => ({ resolveOrgRegion: vi.fn(async () => "https://us.sentry.io"), @@ -47,7 +45,7 @@ vi.mock("../../../src/lib/api/chunk-upload.js", async (importOriginal) => { concurrency: 8, compression: ["gzip"], })), - uploadMissingBufferChunks: uploadMissingBufferChunksMock, + uploadChunkPayloads: uploadChunkPayloadsMock, }; }); @@ -96,7 +94,7 @@ function setServerOptions(overrides: Record): void { beforeEach(() => { apiRequestToRegionMock.mockReset(); - uploadMissingBufferChunksMock.mockClear(); + uploadChunkPayloadsMock.mockClear(); vi.mocked(getChunkUploadOptions).mockResolvedValue( BASE_SERVER_OPTIONS as Awaited>, ); @@ -212,7 +210,7 @@ describe("uploadDebugFiles", () => { await vi.runAllTimersAsync(); const results = await promise; - expect(uploadMissingBufferChunksMock).toHaveBeenCalled(); + expect(uploadChunkPayloadsMock).toHaveBeenCalled(); expect(apiRequestToRegionMock).toHaveBeenCalledTimes(2); expect(results[0]?.state).toBe("created"); }); @@ -418,10 +416,10 @@ describe("uploadDebugFiles", () => { // First call has the file's entry missing from response, so all its // chunks must have been queued for re-upload. - expect(uploadMissingBufferChunksMock).toHaveBeenCalled(); - const firstCall = uploadMissingBufferChunksMock.mock.calls[0]?.[0]; - const passedMissing = firstCall?.missingChecksums as Set; - expect(passedMissing.size).toBeGreaterThan(0); + expect(uploadChunkPayloadsMock).toHaveBeenCalled(); + const firstCall = uploadChunkPayloadsMock.mock.calls[0]?.[0]; + const passedChunks = firstCall?.chunks as unknown[]; + expect(passedChunks.length).toBeGreaterThan(0); }); test("re-sends every chunk when entry state is not_found without missingChunks", async () => { @@ -448,10 +446,10 @@ describe("uploadDebugFiles", () => { maxWaitMs: 60_000, }); - const firstCall = uploadMissingBufferChunksMock.mock.calls[0]?.[0]; - const passedMissing = firstCall?.missingChecksums as Set; + const firstCall = uploadChunkPayloadsMock.mock.calls[0]?.[0]; + const passedChunks = firstCall?.chunks as unknown[]; // Must contain the file's chunk checksums, not be empty. - expect(passedMissing.size).toBeGreaterThan(0); + expect(passedChunks.length).toBeGreaterThan(0); }); test("does not assemble oversized files but reports them as failures", async () => { From 39e37f9070f2c870a28b477ba8a8d5c45e4c8050 Mon Sep 17 00:00:00 2001 From: Ivan Tustanivskyi Date: Wed, 4 Nov 2026 17:03:59 +0200 Subject: [PATCH 3/3] fix(cli): Treat a bare 32-hex debug id as its hyphenated form `--id` could select a DXIL shader by its 32-hex file stem, but `--require-all` compared the same value against the hyphenated id and reported it missing. normalizeDebugId now hyphenates a bare 32-hex id, so filtering, --require-all and find agree, and the shader-specific stem match is gone. --- packages/cli/src/lib/dif/scan.ts | 33 +++++++++++++------------- packages/cli/test/lib/dif/dxil.test.ts | 15 ++++++++++++ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/lib/dif/scan.ts b/packages/cli/src/lib/dif/scan.ts index 492bde995..c93decd57 100644 --- a/packages/cli/src/lib/dif/scan.ts +++ b/packages/cli/src/lib/dif/scan.ts @@ -64,7 +64,7 @@ const TYPE_TO_FORMAT: Readonly> = { const DXIL_HEADER = Buffer.from("DXBC", "ascii"); /** A DXIL file is named after its 32-hex compiler hash, which is its debug id. */ -const DXIL_STEM_RE = /^[0-9a-f]{32}$/i; +const BARE_HEX32_RE = /^[0-9a-f]{32}$/; /** Accepted `--type` filter values. */ export const VALID_DIF_TYPES: readonly string[] = Object.keys(TYPE_TO_FORMAT); @@ -129,7 +129,17 @@ export type PreparedDif = { * @returns The normalized form. */ export function normalizeDebugId(id: string): string { - return id.trim().toLowerCase().replace(/[{}]/g, ""); + const normalized = id.trim().toLowerCase().replace(/[{}]/g, ""); + // A bare 32-hex id (an Apple UUID without hyphens, or a DXIL shader's file + // stem) is the hyphenated id with the hyphens dropped; compare them as one. + return BARE_HEX32_RE.test(normalized) + ? hyphenateHex32(normalized) + : normalized; +} + +/** `xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx` from 32 hex digits. */ +function hyphenateHex32(hex: string): string { + return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`; } /** @@ -730,12 +740,6 @@ function peekBufferFormat(content: Buffer): string { return detectFormat(new Uint8Array(content.subarray(0, PEEK_HEADER_BYTES))); } -/** The hyphenated debug id of a DXIL file named after its 32-hex compiler hash. */ -function dxilDebugId(hex: string): string { - const h = hex.toLowerCase(); - return `${h.slice(0, 8)}-${h.slice(8, 12)}-${h.slice(12, 16)}-${h.slice(16, 20)}-${h.slice(20)}`; -} - /** * Prepare a DXIL shader binary. Its debug id is the compiler hash the engine * uses as the file name; the code id and shader group come from the manifest @@ -748,20 +752,17 @@ function dxilDif( filters: DifFilters, info?: ShaderInfo, ): PreparedDif[] { - const stem = basename(displayPath, extname(displayPath)); - if (!DXIL_STEM_RE.test(stem)) { + const stem = basename(displayPath, extname(displayPath)).toLowerCase(); + if (!BARE_HEX32_RE.test(stem)) { log.warn( `Skipping ${displayPath}: a DXIL file must be named after its 32-hex shader hash`, ); return []; } - const debugId = dxilDebugId(stem); - // `--id` may be given as the bare file stem or in the hyphenated form. + // normalizeDebugId hyphenates the stem, so `--id` matches either spelling. + const debugId = normalizeDebugId(stem); if ( - !( - formatMatches("dxil", filters.formats) && - (idMatches(debugId, filters.ids) || idMatches(stem, filters.ids)) - ) + !(formatMatches("dxil", filters.formats) && idMatches(debugId, filters.ids)) ) { return []; } diff --git a/packages/cli/test/lib/dif/dxil.test.ts b/packages/cli/test/lib/dif/dxil.test.ts index 3d8c11902..d17bf3df0 100644 --- a/packages/cli/test/lib/dif/dxil.test.ts +++ b/packages/cli/test/lib/dif/dxil.test.ts @@ -11,6 +11,8 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { buildDifFilters, + debugIdMatches, + normalizeDebugId, prepareDifs, scanPaths, } from "../../../src/lib/dif/scan.js"; @@ -50,6 +52,19 @@ async function writeShaderFixture( return path; } +describe("normalizeDebugId with a bare 32-hex id", () => { + test("hyphenates it so both spellings compare equal", () => { + const stem = DXIL_NAME.replace(/\.dxil$/, ""); + expect(normalizeDebugId(stem.toUpperCase())).toBe(DXIL_DEBUG_ID); + expect(normalizeDebugId(DXIL_DEBUG_ID)).toBe(DXIL_DEBUG_ID); + expect(debugIdMatches(stem, DXIL_DEBUG_ID)).toBe(true); + // Shorter or longer hex strings (a Breakpad id carries an age digit) are left alone. + expect(normalizeDebugId("0F13A5DA412AFBF7C8662048F3294F3D0")).toBe( + "0f13a5da412afbf7c8662048f3294f3d0", + ); + }); +}); + describe("prepareDifs with DXIL shader binaries", () => { test("recognises a shader by its header and names it from the file stem", async () => { const path = await writeShaderFixture(DXIL_NAME, { manifest: true });