diff --git a/docs/src/fragments/commands/debug-files.md b/docs/src/fragments/commands/debug-files.md index 90986a2a4..82574a228 100644 --- a/docs/src/fragments/commands/debug-files.md +++ b/docs/src/fragments/commands/debug-files.md @@ -60,11 +60,14 @@ sentry debug-files upload ./build --no-upload files via the chunk-upload protocol. Use `--type`/`--id` to restrict which files are sent, `--no-debug`/`--no-unwind`/`--no-sources` to drop files whose only useful feature is the named one, and `--include-sources` to attach a - source bundle per file. `--no-upload` previews the selection without - credentials; `--wait`/`--wait-for` block on server-side processing and exit - non-zero if any file fails. `--require-all` fails if a requested `--id` was not - found. Scanning inside ZIP archives, `--symbol-maps`, `--il2cpp-mapping` line - mappings, and `--derived-data` are not yet supported. + source bundle per file. `--derived-data` additionally scans Xcode's + `~/Library/Developer/Xcode/DerivedData` folder (macOS only). `--no-upload` + previews the selection without credentials; `--wait`/`--wait-for` block on + server-side processing and exit non-zero if any file fails. `--require-all` + fails if a requested `--id` was not found. The server-advertised maximum file + size and maximum processing wait are honored automatically (oversized files + are skipped with a warning). Scanning inside ZIP archives, `--symbol-maps`, + and `--il2cpp-mapping` line mappings are not yet supported. - Upload a JVM bundle separately via `sentry debug-files upload --type jvm`. - Supported JVM source file extensions: `.java`, `.kt`, `.scala`, `.sc`, `.groovy`, `.gvy`, `.gy`, `.gsh`, `.clj`, `.cljc` diff --git a/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md b/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md index 210a0f19d..412330f72 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md @@ -27,6 +27,7 @@ Upload debug information files to Sentry - `--no-unwind - Do not upload files whose only feature is unwind info` - `--no-sources - Do not upload files whose only feature is source info` - `--include-sources - Build and upload a source bundle for each file with debug info` +- `--derived-data - Also scan Xcode's DerivedData folder (macOS only)` - `--no-upload - Scan and print what would be uploaded without uploading` - `--wait - Wait for server-side processing and report any errors` - `--wait-for - Wait up to this many seconds for server-side processing` diff --git a/src/commands/debug-files/upload.ts b/src/commands/debug-files/upload.ts index 2c372b365..0ee065815 100644 --- a/src/commands/debug-files/upload.ts +++ b/src/commands/debug-files/upload.ts @@ -8,15 +8,23 @@ * Org/project are resolved via the standard cascade (DSN auto-detection, env * vars, config defaults), so `--no-upload` (dry-run) needs no credentials. * - * This is the first stage of `debug-files upload` parity. ZIP scanning, - * `--symbol-maps`, `--il2cpp-mapping` line mappings, and `--derived-data` are - * deferred to follow-up PRs (see the command's full description). + * Honors the server-advertised `max_file_size` (oversized files are skipped) + * and `max_wait` (clamps the processing wait). `--derived-data` additionally + * scans Xcode's DerivedData folder on macOS. ZIP scanning, `--symbol-maps`, + * and `--il2cpp-mapping` line mappings are deferred to follow-up PRs (see the + * command's full description). */ import { createHash } from "node:crypto"; -import { readFileSync } from "node:fs"; -import { basename } from "node:path"; +import { existsSync, readFileSync } from "node:fs"; +import { homedir } from "node:os"; +import { basename, join } from "node:path"; import type { SentryContext } from "../../context.js"; +import { + type ChunkServerOptions, + DEFAULT_MAX_DIF_SIZE, + getChunkUploadOptions, +} from "../../lib/api/chunk-upload.js"; import { DEBUG_FILES_MAX_WAIT_MS, type DebugFileUpload, @@ -46,6 +54,40 @@ const log = logger.withTag("debug-files.upload"); const USAGE_HINT = "sentry debug-files upload ..."; +/** Relative path to Xcode's DerivedData folder under the user's home dir. */ +const DERIVED_DATA_SUBPATH = "Library/Developer/Xcode/DerivedData"; + +/** + * Resolve the effective scan paths, optionally appending Xcode's DerivedData + * folder when `--derived-data` is set. + * + * DerivedData only exists on macOS; on other platforms the flag is a no-op + * (with a warning). The folder is appended only when it actually exists, so the + * stricter `scanPaths` existence check (which throws on a missing explicit + * path) is never tripped by an absent DerivedData directory. + * + * @param paths - Positional paths supplied on the command line. + * @param derivedData - Whether `--derived-data` was passed. + * @returns The effective list of paths to scan. + */ +function collectScanPaths(paths: string[], derivedData: boolean): string[] { + if (!derivedData) { + return paths; + } + if (process.platform !== "darwin") { + log.warn("--derived-data is only supported on macOS; ignoring it."); + return paths; + } + const derivedDataPath = join(homedir(), DERIVED_DATA_SUBPATH); + if (!existsSync(derivedDataPath)) { + log.warn( + `Xcode DerivedData folder not found at ${derivedDataPath}; ignoring --derived-data.` + ); + return paths; + } + return [...paths, derivedDataPath]; +} + // ── Types ─────────────────────────────────────────────────────────── /** Per-file entry in the command result. */ @@ -83,6 +125,7 @@ type UploadFlags = { "no-unwind"?: boolean; "no-sources"?: boolean; "include-sources"?: boolean; + "derived-data"?: boolean; "no-upload"?: boolean; wait?: boolean; "wait-for"?: number; @@ -253,10 +296,14 @@ function resolveWaitMode(flags: UploadFlags): { */ function* doDryRun( setExitCode: (code: number) => void, - difs: DebugFileUpload[], - missingIds: string[], - requireAll: boolean + params: { + difs: DebugFileUpload[]; + missingIds: string[]; + requireAll: boolean; + oversizedCount: number; + } ) { + const { difs, missingIds, requireAll, oversizedCount } = params; yield new CommandOutput({ uploaded: false, files: difs.map((d) => ({ name: d.name, debugId: d.debugId })), @@ -266,11 +313,20 @@ function* doDryRun( setExitCode(1); return { hint: `Missing requested debug id(s): ${missingIds.join(", ")}` }; } + if (difs.length > 0) { + return { + hint: `Would upload ${difs.length} debug file(s). Remove --no-upload to upload.`, + }; + } + // Distinguish "nothing matched" from "files were skipped for size" so a + // dry-run does not misleadingly report an empty scan. The count reflects + // size-skipped files of a requested type; it does not claim that was the + // only reason nothing would upload. return { hint: - difs.length === 0 - ? `No debug information files found. Try: ${USAGE_HINT}` - : `Would upload ${difs.length} debug file(s). Remove --no-upload to upload.`, + oversizedCount > 0 + ? `${oversizedCount} file(s) would be skipped for exceeding the maximum file size.` + : `No debug information files found. Try: ${USAGE_HINT}`, }; } @@ -280,19 +336,35 @@ function* doDryRun( */ function* doNothingToUpload( setExitCode: (code: number) => void, - missingIds: string[], - requireAll: boolean + params: { + missingIds: string[]; + requireAll: boolean; + oversizedCount: number; + maxFileSize: number; + } ) { - log.warn("No debug information files found."); + const { missingIds, requireAll, oversizedCount, maxFileSize } = params; yield new CommandOutput({ uploaded: false, files: [], filesUploaded: 0, }); + // --require-all takes precedence: a requested id that wasn't found is the + // most actionable failure, regardless of why the queue ended up empty. if (missingIds.length > 0 && requireAll) { setExitCode(1); return { hint: `Missing requested debug id(s): ${missingIds.join(", ")}` }; } + // Files of a requested type were found but skipped for size. Fail non-zero + // with an accurate count (this does not claim it was the *only* reason the + // queue is empty — other candidates may have failed id/feature filters). + if (oversizedCount > 0) { + setExitCode(1); + return { + hint: `No debug files were uploaded: ${oversizedCount} file(s) exceeded the maximum file size (${maxFileSize} bytes).`, + }; + } + log.warn("No debug information files found."); return { hint: `No debug information files found. Try: ${USAGE_HINT}` }; } @@ -311,6 +383,7 @@ async function* doUpload( maxWaitMs: number; missingRequestedIds: string[]; requireAll: boolean; + serverOptions?: ChunkServerOptions; } ) { const results = await uploadDebugFiles(params); @@ -374,15 +447,17 @@ export const uploadCommand = buildCommand({ " sourcebundle, jvm\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\n" + + " useful feature is the named one\n" + + " --derived-data Also scan Xcode's DerivedData folder (macOS only)\n\n" + "Usage:\n" + " sentry debug-files upload ./build\n" + " sentry debug-files upload ./libexample.so --include-sources\n" + " sentry debug-files upload ./dsyms --type dsym --wait\n" + + " sentry debug-files upload --derived-data --no-upload\n" + " sentry debug-files upload ./build --no-upload\n\n" + "Not yet supported (planned): scanning inside ZIP archives, " + - "--symbol-maps (BCSymbolMap resolution), --il2cpp-mapping line " + - "mappings, and --derived-data.", + "--symbol-maps (BCSymbolMap resolution), and --il2cpp-mapping line " + + "mappings.", }, output: { human: formatUploadResult, @@ -443,6 +518,12 @@ export const uploadCommand = buildCommand({ optional: true, default: false, }, + "derived-data": { + kind: "boolean", + brief: "Also scan Xcode's DerivedData folder (macOS only)", + optional: true, + default: false, + }, "no-upload": { kind: "boolean", brief: "Scan and print what would be uploaded without uploading", @@ -467,10 +548,15 @@ export const uploadCommand = buildCommand({ }, }, async *func(this: SentryContext, flags: UploadFlags, ...paths: string[]) { - if (paths.length === 0) { + const scanTargets = collectScanPaths(paths, Boolean(flags["derived-data"])); + if (scanTargets.length === 0) { throw new ContextError("Debug file path(s)", USAGE_HINT, []); } const { wait, maxWaitMs } = resolveWaitMode(flags); + const requireAll = Boolean(flags["require-all"]); + const setExitCode = (c: number) => { + this.process.exitCode = c; + }; const filters = buildDifFilters({ types: flags.type, @@ -479,56 +565,72 @@ export const uploadCommand = buildCommand({ noUnwind: flags["no-unwind"], noSources: flags["no-sources"], }); - const files = await scanPaths(paths); - const prepared = await prepareDifs(files, filters); + + // For a real upload, resolve org/project and fetch the server's upload + // options up front. The server's advertised `maxFileSize` then gates the + // scan, so a file the server would reject is never read into memory. + // `--no-upload` stays auth-free and uses the generous default cap. + let resolved: Awaited> = null; + let serverOptions: ChunkServerOptions | undefined; + let maxFileSize = DEFAULT_MAX_DIF_SIZE; + if (!flags["no-upload"]) { + resolved = await resolveOrgAndProject({ + cwd: this.cwd, + usageHint: USAGE_HINT, + }); + if (!resolved) { + throw new ContextError("Organization and project", USAGE_HINT); + } + serverOptions = await getChunkUploadOptions(resolved.org); + if (serverOptions.maxFileSize && serverOptions.maxFileSize > 0) { + maxFileSize = serverOptions.maxFileSize; + } + } + + const files = await scanPaths(scanTargets); + const { prepared, oversizedCount } = await prepareDifs(files, filters, { + maxFileSize, + }); const difs = dedupeDifs( buildDifList(prepared, Boolean(flags["include-sources"])) ); const missingIds = missingRequestedIds(flags.id, prepared); - const requireAll = Boolean(flags["require-all"]); + // Dry-run is purely informational: report what would upload (and surface + // size skips) without erroring. if (flags["no-upload"]) { - return yield* doDryRun( - (c) => { - this.process.exitCode = c; - }, + return yield* doDryRun(setExitCode, { difs, missingIds, - requireAll - ); + requireAll, + oversizedCount, + }); } if (difs.length === 0) { - return yield* doNothingToUpload( - (c) => { - this.process.exitCode = c; - }, + return yield* doNothingToUpload(setExitCode, { missingIds, - requireAll - ); + requireAll, + oversizedCount, + maxFileSize, + }); } - const resolved = await resolveOrgAndProject({ - cwd: this.cwd, - usageHint: USAGE_HINT, - }); + // `resolved` is guaranteed set here: the non-dry-run branch above resolved + // it or threw. if (!resolved) { throw new ContextError("Organization and project", USAGE_HINT); } - return yield* doUpload( - (c) => { - this.process.exitCode = c; - }, - { - org: resolved.org, - project: resolved.project, - difs, - wait, - maxWaitMs, - missingRequestedIds: missingIds, - requireAll, - } - ); + return yield* doUpload(setExitCode, { + org: resolved.org, + project: resolved.project, + serverOptions, + difs, + wait, + maxWaitMs, + missingRequestedIds: missingIds, + requireAll, + }); }, }); diff --git a/src/lib/api/chunk-upload.ts b/src/lib/api/chunk-upload.ts index e9a2da312..2a6b285d3 100644 --- a/src/lib/api/chunk-upload.ts +++ b/src/lib/api/chunk-upload.ts @@ -48,6 +48,19 @@ export const ChunkServerOptionsSchema = z.object({ chunksPerRequest: z.number(), /** Maximum total request body size in bytes. */ maxRequestSize: z.number(), + /** + * Maximum size of a single uploaded file in bytes. Omitted or `0` means the + * server advertises no per-file cap, in which case the client falls back to + * {@link DEFAULT_MAX_DIF_SIZE}. + */ + maxFileSize: z.number().optional(), + /** + * Maximum time, in seconds, the server is willing to spend assembling an + * upload. Omitted or `0` means no server-imposed cap; a non-zero value clamps + * the caller's requested wait. Mirrors the legacy `dif_upload` `max_wait` + * semantics. + */ + maxWait: z.number().optional(), /** Hash algorithm for chunk checksums (always "sha1"). */ hashAlgorithm: z.string(), /** Maximum concurrent upload requests. */ @@ -87,6 +100,12 @@ export const ASSEMBLE_POLL_INTERVAL_MS = 1000; /** Maximum time to wait for assembly. */ export const ASSEMBLE_MAX_WAIT_MS = 300_000; +/** + * Fallback per-file size cap (2 GiB) used when the server advertises no + * `maxFileSize` (i.e. reports `0`). Matches the legacy `DEFAULT_MAX_DIF_SIZE`. + */ +export const DEFAULT_MAX_DIF_SIZE = 2 * 1024 * 1024 * 1024; + /** * Codecs the CLI knows how to emit, in order of preference. * diff --git a/src/lib/api/debug-files.ts b/src/lib/api/debug-files.ts index f4ae2d9e8..0921f47d7 100644 --- a/src/lib/api/debug-files.ts +++ b/src/lib/api/debug-files.ts @@ -19,7 +19,7 @@ */ import { z } from "zod"; -import { ApiError } from "../errors.js"; +import { ApiError, ValidationError } from "../errors.js"; import { logger } from "../logger.js"; import { resolveOrgRegion } from "../region.js"; import { @@ -29,6 +29,7 @@ import { AssembleResponseSchema, type ChunkInfo, type ChunkServerOptions, + DEFAULT_MAX_DIF_SIZE, getChunkUploadOptions, hashBuffer, pickUploadEncoding, @@ -71,6 +72,12 @@ export type DebugFilesUploadOptions = { wait: boolean; /** Maximum time to wait for assembly/processing, in milliseconds. */ maxWaitMs: number; + /** + * Pre-fetched chunk-upload server options. When provided, the internal + * `getChunkUploadOptions` call is skipped (the caller already fetched them, + * e.g. to size-gate the scan against the server's `maxFileSize`). + */ + serverOptions?: ChunkServerOptions; }; /** Per-file result of a debug information file upload. */ @@ -292,6 +299,54 @@ function buildResults( // ── API Functions ─────────────────────────────────────────────────── +/** + * Drop files larger than the server-advertised per-file cap. + * + * @param difs - Files queued for upload. + * @param maxFileSize - Server's per-file byte cap; `0` disables the gate. + * @returns The subset within the cap. Oversized files are logged at `warn`. + */ +function filterBySize( + difs: DebugFileUpload[], + maxFileSize: number +): DebugFileUpload[] { + if (maxFileSize <= 0) { + return difs; + } + const accepted: DebugFileUpload[] = []; + for (const dif of difs) { + if (dif.content.length > maxFileSize) { + log.warn( + `Skipping ${dif.name}: size ${dif.content.length} exceeds server maximum file size ${maxFileSize}` + ); + continue; + } + accepted.push(dif); + } + return accepted; +} + +/** + * Clamp the caller's requested wait to the server's advertised maximum. + * + * @param requestedMs - The caller's requested wait in milliseconds. + * @param serverMaxWaitSec - Server's max wait in seconds; `0` means no cap. + * @returns The effective wait in milliseconds. + */ +function clampMaxWait(requestedMs: number, serverMaxWaitSec: number): number { + if (serverMaxWaitSec <= 0) { + return requestedMs; + } + const serverMaxMs = serverMaxWaitSec * 1000; + if (serverMaxMs < requestedMs) { + log.debug( + `Clamping assembly wait from ${requestedMs}ms to server maximum ${serverMaxMs}ms` + ); + return serverMaxMs; + } + return requestedMs; +} + /** * Upload debug information files to Sentry via the DIF chunk-upload protocol. * @@ -300,10 +355,13 @@ function buildResults( * The server re-parses each uploaded file and indexes every contained object * slice itself; `debugId` is advisory. * + * Files larger than the server-advertised `maxFileSize` are dropped with a + * warning, and the caller's `maxWaitMs` is clamped to the server's `maxWait`. + * * @param options - Upload configuration (org, project, files, wait mode). * @returns Per-file terminal/last-observed assembly state. * @throws {ApiError} If chunk upload fails, or (wait mode only) assembly does - * not complete within `maxWaitMs`. + * not complete within the effective (clamped) wait. */ export async function uploadDebugFiles( options: DebugFilesUploadOptions @@ -314,10 +372,32 @@ export async function uploadDebugFiles( return []; } - const serverOptions = await getChunkUploadOptions(org); + const serverOptions = + options.serverOptions ?? (await getChunkUploadOptions(org)); const encoding = pickUploadEncoding(serverOptions.compression); - const chunked: ChunkedDif[] = difs.map((dif) => { + // Honor the per-file cap. Use the server-advertised `maxFileSize`, falling + // back to `DEFAULT_MAX_DIF_SIZE` when the server omits it (`0`) — the same + // fallback the scan gate uses, so in-memory `--include-sources` bundles + // (which bypass the scan gate) are still capped here. + const effectiveMaxFileSize = + serverOptions.maxFileSize && serverOptions.maxFileSize > 0 + ? serverOptions.maxFileSize + : DEFAULT_MAX_DIF_SIZE; + const accepted = filterBySize(difs, effectiveMaxFileSize); + + // `difs` is non-empty (checked above), so an empty `accepted` means every + // file was dropped by the size gate. Fail loudly instead of silently + // reporting success with nothing uploaded. + if (accepted.length === 0) { + throw new ValidationError( + `All ${difs.length} debug file(s) exceed the maximum file size ` + + `(${effectiveMaxFileSize} bytes). Nothing was uploaded.`, + "file" + ); + } + + const chunked: ChunkedDif[] = accepted.map((dif) => { const { chunks, overallChecksum } = hashBuffer( dif.content, serverOptions.chunkSize @@ -329,7 +409,13 @@ export async function uploadDebugFiles( const endpoint = `projects/${org}/${project}/files/difs/assemble/`; const body = buildAssembleBody(chunked); - const deadline = Date.now() + maxWaitMs; + // 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 + // the chunk-delivery loop, not a processing budget, so it is left untouched. + const effectiveMaxWaitMs = wait + ? clampMaxWait(maxWaitMs, serverOptions.maxWait ?? 0) + : maxWaitMs; + const deadline = Date.now() + effectiveMaxWaitMs; let response = await postAssemble(regionUrl, endpoint, body); let evaluation = evaluateAssembly(response, chunked, wait); await uploadMissing({ @@ -346,7 +432,7 @@ export async function uploadDebugFiles( throw new ApiError( "Debug file assembly timed out", 408, - `Assembly did not complete within ${Math.round(maxWaitMs / 1000)}s`, + `Assembly did not complete within ${Math.round(effectiveMaxWaitMs / 1000)}s`, endpoint ); } diff --git a/src/lib/dif/scan.ts b/src/lib/dif/scan.ts index d7d9354f0..a36f8c835 100644 --- a/src/lib/dif/scan.ts +++ b/src/lib/dif/scan.ts @@ -330,25 +330,43 @@ export async function scanPaths(paths: string[]): Promise { const PEEK_HEADER_BYTES = 4096; +/** A candidate file's recognised format and total size in bytes. */ +type PeekResult = { + /** + * The container format detected from the header (canonical name, e.g. + * `elf`, `macho`, `pe`). Never `unknown` (those return `null`). + */ + format: string; + /** Total file size in bytes (from the open descriptor's `stat`). */ + size: number; +}; + /** * Peek at a file's header bytes for format detection — avoids reading the - * whole file for large non-DIF data. Returns `null` when the file is - * unreadable, empty, or in an unrecognised format. + * whole file for large non-DIF data. Also reports the total file size (from + * the open descriptor, so no extra `stat` syscall) so callers can gate on + * format and size before fully reading the file. Returns `null` when the file + * is unreadable, empty, or in an unrecognised format. */ -async function peekHeader(path: string): Promise { +async function peekHeader(path: string): Promise { try { const fd = await open(path, "r"); try { + const { size } = await fd.stat(); const buf = Buffer.alloc(PEEK_HEADER_BYTES); const { bytesRead } = await fd.read(buf, 0, PEEK_HEADER_BYTES, 0); const header = bytesRead < PEEK_HEADER_BYTES ? new Uint8Array(buf.subarray(0, bytesRead)) : new Uint8Array(buf); - if (header.length === 0 || peekFormat(header) === "unknown") { + if (header.length === 0) { return null; } - return header; + const format = peekFormat(header); + if (format === "unknown") { + return null; + } + return { format, size }; } finally { await fd.close(); } @@ -358,6 +376,28 @@ async function peekHeader(path: string): Promise { } } +/** Options controlling {@link prepareDifs} behavior. */ +export type PrepareDifsOptions = { + /** + * Maximum size, in bytes, of a file to keep. Files larger than this are + * skipped with a warning. `0` or omitted means no size gate. Mirrors the + * legacy `dif_upload` `valid_size` check. + */ + maxFileSize?: number; +}; + +/** Result of {@link prepareDifs}: the uploadable files plus skip telemetry. */ +export type PrepareDifsResult = { + /** The files to upload, each with its matched objects and primary id. */ + prepared: PreparedDif[]; + /** + * Number of recognised debug files skipped solely because they exceeded + * `maxFileSize`. Lets callers distinguish "nothing found" from "everything + * found was too large" so they can fail with an accurate message. + */ + oversizedCount: number; +}; + /** * Read, parse, and filter candidate files into uploadable debug files. * @@ -372,53 +412,96 @@ async function peekHeader(path: string): Promise { * * @param paths - Candidate file paths (from {@link scanPaths}). * @param filters - The resolved filter set. - * @returns The files to upload, each with its matched objects and primary id. + * @param options - Optional size gate (see {@link PrepareDifsOptions}). + * @returns The uploadable files plus the count of size-skipped files + * (see {@link PrepareDifsResult}). */ export async function prepareDifs( paths: string[], - filters: DifFilters -): Promise { + filters: DifFilters, + options: PrepareDifsOptions = {} +): Promise { const prepared: PreparedDif[] = []; + const maxFileSize = options.maxFileSize ?? 0; + let oversizedCount = 0; for (const path of paths) { - if (!(await peekHeader(path))) { + const peeked = await peekHeader(path); + if (!peeked) { continue; } - - let content: Buffer; - try { - content = await readFile(path); - } catch (err) { - log.debug(`Skipping unreadable file: ${path}`, err); + // Apply the cheap, header-derivable `--type` (format) filter before the + // size gate. This keeps `oversizedCount` aligned with the requested type + // so an oversized file of an unrequested format never triggers an + // "all matched files too large" outcome. Per-object `--id`/feature filters + // still require a full parse and run in {@link readMatchedDif}. + if (!formatMatches(peeked.format, filters.formats)) { continue; } - if (content.length === 0) { + // Gate on size before the full read so an oversized file is never buffered. + // Only recognised debug files of a requested format reach here, so an + // oversized skip means a real, requested DIF was too large. + if (maxFileSize > 0 && peeked.size > maxFileSize) { + oversizedCount += 1; + log.warn( + `Skipping ${path}: size ${peeked.size} exceeds maximum file size ${maxFileSize}` + ); continue; } - const data = new Uint8Array(content); - let archive: DifArchiveInfo; - try { - archive = parseDebugFile(data); - } catch (err) { - log.debug(`Skipping unparseable file: ${path}`, err); - continue; + const dif = await readMatchedDif(path, filters); + if (dif) { + prepared.push(dif); } + } - const matched = archive.objects.filter((obj) => - objectPassesFilters(obj, filters) - ); - if (matched.length === 0) { - continue; - } + return { prepared, oversizedCount }; +} + +/** + * Fully read and parse a single candidate file, returning it as a + * {@link PreparedDif} when at least one contained object passes the per-object + * filters, or `null` when the file is unreadable, unparseable, empty, or has no + * matching object. Read/parse failures are logged at debug level — a scanned + * tree contains many non-object files. + * + * @param path - The candidate file path (already format- and size-gated). + * @param filters - The resolved filter set. + */ +async function readMatchedDif( + path: string, + filters: DifFilters +): Promise { + let content: Buffer; + try { + content = await readFile(path); + } catch (err) { + log.debug(`Skipping unreadable file: ${path}`, err); + return null; + } + if (content.length === 0) { + return null; + } + + let archive: DifArchiveInfo; + try { + archive = parseDebugFile(new Uint8Array(content)); + } catch (err) { + log.debug(`Skipping unparseable file: ${path}`, err); + return null; + } - prepared.push({ - path, - content, - debugId: selectBundledObject(matched)?.debugId, - objects: matched, - }); + const matched = archive.objects.filter((obj) => + objectPassesFilters(obj, filters) + ); + if (matched.length === 0) { + return null; } - return prepared; + return { + path, + content, + debugId: selectBundledObject(matched)?.debugId, + objects: matched, + }; } diff --git a/test/commands/debug-files/upload.test.ts b/test/commands/debug-files/upload.test.ts index 3d7b76c9e..7a88685f1 100644 --- a/test/commands/debug-files/upload.test.ts +++ b/test/commands/debug-files/upload.test.ts @@ -15,6 +15,8 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { app } from "../../../src/app.js"; import type { SentryContext } from "../../../src/context.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as chunkUpload from "../../../src/lib/api/chunk-upload.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as debugFilesApi from "../../../src/lib/api/debug-files.js"; import { useTestConfigDir } from "../../helpers.js"; @@ -39,6 +41,17 @@ beforeEach(async () => { SENTRY_ORG: process.env.SENTRY_ORG, SENTRY_PROJECT: process.env.SENTRY_PROJECT, }; + // The real-upload path fetches chunk-upload options before scanning to gate + // on the server's max file size. Stub it so tests need no network. + vi.spyOn(chunkUpload, "getChunkUploadOptions").mockResolvedValue({ + url: "https://us.sentry.io/api/0/chunk-upload/", + chunkSize: 8192, + chunksPerRequest: 64, + maxRequestSize: 1_048_576, + hashAlgorithm: "sha1", + concurrency: 8, + compression: ["gzip"], + } as Awaited>); }); afterEach(async () => { @@ -124,6 +137,30 @@ describe("sentry debug-files upload", () => { expect(exitCode).not.toBe(0); }); + // ── --derived-data ─────────────────────────────────────────────── + + test("--derived-data is additive: explicit paths still scan", async () => { + await writeBreakpad(); + const { output, exitCode } = await runUpload([ + tempDir, + "--derived-data", + "--no-upload", + "--json", + ]); + expect(exitCode).toBe(0); + // The explicit tempDir fixture is found regardless of whether a + // DerivedData folder exists on this platform. + expect(JSON.parse(output).files).toHaveLength(1); + }); + + test.skipIf(process.platform === "darwin")( + "--derived-data alone on non-macOS exits non-zero (no scan targets)", + async () => { + const { exitCode } = await runUpload(["--derived-data", "--no-upload"]); + expect(exitCode).not.toBe(0); + } + ); + // ── --no-upload (dry-run) ──────────────────────────────────────── test("--no-upload scans a directory and reports the debug id", async () => { @@ -267,9 +304,37 @@ describe("sentry debug-files upload", () => { expect(callArgs?.difs).toHaveLength(1); expect(callArgs?.difs[0]?.debugId).toBe(KNOWN_DEBUG_ID); expect(callArgs?.difs[0]?.content).toBeInstanceOf(Buffer); + // The server options fetched for the scan gate are threaded through so + // uploadDebugFiles does not re-fetch them. + expect(callArgs?.serverOptions).toBeDefined(); expect(exitCode).toBe(0); }); + test("server maxFileSize gating all files exits non-zero with a clear error", async () => { + process.env.SENTRY_ORG = "test-org"; + process.env.SENTRY_PROJECT = "test-project"; + await writeBreakpad(); + // Advertise a 10-byte cap; the fixture is well over that, so it is skipped + // during the scan and never reaches uploadDebugFiles. Because a real DIF + // was found but dropped for size, the command must fail loudly rather than + // reporting "nothing found". + vi.spyOn(chunkUpload, "getChunkUploadOptions").mockResolvedValue({ + url: "https://us.sentry.io/api/0/chunk-upload/", + chunkSize: 8192, + chunksPerRequest: 64, + maxRequestSize: 1_048_576, + maxFileSize: 10, + hashAlgorithm: "sha1", + concurrency: 8, + compression: ["gzip"], + } as Awaited>); + const spy = vi.spyOn(debugFilesApi, "uploadDebugFiles"); + + const { exitCode } = await runUpload([tempDir]); + expect(spy).not.toHaveBeenCalled(); + expect(exitCode).not.toBe(0); + }); + test("wait mode surfaces processing errors with a non-zero exit", async () => { process.env.SENTRY_ORG = "test-org"; process.env.SENTRY_PROJECT = "test-project"; @@ -336,6 +401,26 @@ describe("sentry debug-files upload", () => { expect(output).toContain("11111111-1111-1111-1111-111111111111"); }); + test("--require-all is honored when the queue is empty (real upload)", async () => { + process.env.SENTRY_ORG = "test-org"; + process.env.SENTRY_PROJECT = "test-project"; + await writeBreakpad(); + const spy = vi.spyOn(debugFilesApi, "uploadDebugFiles"); + + // The only --id requested doesn't match the fixture, so the queue is empty. + // doNothingToUpload must still honor --require-all and report the missing + // id (exit 1) rather than skipping the check. + const { exitCode, output } = await runUpload([ + tempDir, + "--id", + "11111111-1111-1111-1111-111111111111", + "--require-all", + ]); + expect(spy).not.toHaveBeenCalled(); + expect(exitCode).toBe(1); + expect(output).toContain("11111111-1111-1111-1111-111111111111"); + }); + test("--id without --require-all does not fail on a real upload", async () => { process.env.SENTRY_ORG = "test-org"; process.env.SENTRY_PROJECT = "test-project"; diff --git a/test/lib/api/chunk-upload.test.ts b/test/lib/api/chunk-upload.test.ts new file mode 100644 index 000000000..d73869283 --- /dev/null +++ b/test/lib/api/chunk-upload.test.ts @@ -0,0 +1,44 @@ +/** + * Tests for the chunk-upload server-options schema. + * + * Focuses on the optional `maxFileSize` / `maxWait` fields added for + * `debug-files upload`: a server response that omits them must still parse + * (backward compatibility), and present values must be carried through. + */ + +import { describe, expect, test } from "vitest"; +import { ChunkServerOptionsSchema } from "../../../src/lib/api/chunk-upload.js"; + +const BASE = { + url: "https://us.sentry.io/api/0/chunk-upload/", + chunkSize: 8192, + chunksPerRequest: 64, + maxRequestSize: 1_048_576, + hashAlgorithm: "sha1", + concurrency: 8, + compression: ["gzip"], +}; + +describe("ChunkServerOptionsSchema", () => { + test("parses a response that omits maxFileSize and maxWait", () => { + const result = ChunkServerOptionsSchema.safeParse(BASE); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.maxFileSize).toBeUndefined(); + expect(result.data.maxWait).toBeUndefined(); + } + }); + + test("carries through present maxFileSize and maxWait", () => { + const result = ChunkServerOptionsSchema.safeParse({ + ...BASE, + maxFileSize: 2_147_483_648, + maxWait: 300, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.maxFileSize).toBe(2_147_483_648); + expect(result.data.maxWait).toBe(300); + } + }); +}); diff --git a/test/lib/api/debug-files.test.ts b/test/lib/api/debug-files.test.ts index 4adb8c327..279005344 100644 --- a/test/lib/api/debug-files.test.ts +++ b/test/lib/api/debug-files.test.ts @@ -8,6 +8,7 @@ */ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { getChunkUploadOptions } from "../../../src/lib/api/chunk-upload.js"; import { type DebugFileUpload, uploadDebugFiles, @@ -73,9 +74,31 @@ const SOLE = { project: "app", }; +/** Base server options the mock returns unless a test overrides it. */ +const BASE_SERVER_OPTIONS = { + url: "https://us.sentry.io/api/0/chunk-upload/", + chunkSize: 8192, + chunksPerRequest: 64, + maxRequestSize: 1_048_576, + hashAlgorithm: "sha1", + concurrency: 8, + compression: ["gzip"], +}; + +/** Override the chunk-upload server options for the current test. */ +function setServerOptions(overrides: Record): void { + vi.mocked(getChunkUploadOptions).mockResolvedValue({ + ...BASE_SERVER_OPTIONS, + ...overrides, + } as Awaited>); +} + beforeEach(() => { apiRequestToRegionMock.mockReset(); uploadMissingBufferChunksMock.mockClear(); + vi.mocked(getChunkUploadOptions).mockResolvedValue( + BASE_SERVER_OPTIONS as Awaited> + ); }); afterEach(() => { @@ -349,4 +372,94 @@ describe("uploadDebugFiles", () => { // Must contain the file's chunk checksums, not be empty. expect(passedMissing.size).toBeGreaterThan(0); }); + + test("drops files larger than the server maxFileSize before assembling", async () => { + setServerOptions({ maxFileSize: 5 }); + apiRequestToRegionMock.mockImplementationOnce( + 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 results = await uploadDebugFiles({ + ...SOLE, + // "small" is 5 bytes (<= cap), "this-one-is-too-long" exceeds it. + difs: [ + makeDif("small.so", "small", "id-ok"), + makeDif("big.so", "this-one-is-too-long", "id-big"), + ], + wait: false, + maxWaitMs: 1000, + }); + + const body = lastAssembleBody(); + const names = Object.values(body).map((e) => e.name); + expect(names).toEqual(["small.so"]); + expect(results).toHaveLength(1); + expect(results[0]?.name).toBe("small.so"); + }); + + test("throws when every file exceeds the server maxFileSize", async () => { + setServerOptions({ maxFileSize: 1 }); + + await expect( + uploadDebugFiles({ + ...SOLE, + difs: [makeDif("big.so", "way too large", "id-big")], + wait: false, + maxWaitMs: 1000, + }) + ).rejects.toThrow(/exceed the maximum file size/); + expect(apiRequestToRegionMock).not.toHaveBeenCalled(); + }); + + test("clamps the wait to the server maxWait (reflected in the timeout)", async () => { + setServerOptions({ maxWait: 1 }); + vi.useFakeTimers(); + // Always "assembling" → never terminal. The clamped 1s deadline trips + // well before the requested 60s. + apiRequestToRegionMock.mockImplementation( + async (_url: string, _endpoint: string, init: { body: object }) => { + const key = Object.keys(init.body as object)[0] as string; + return { data: { [key]: { state: "assembling" } } }; + } + ); + + const promise = uploadDebugFiles({ + ...SOLE, + difs: [makeDif("slow.so", "debug bytes", "id-slow")], + wait: true, + maxWaitMs: 60_000, + }).catch((caught: unknown) => caught); + await vi.runAllTimersAsync(); + const err = await promise; + + expect(err).toBeInstanceOf(ApiError); + expect((err as ApiError).detail).toContain("within 1s"); + }); + + test("does not clamp when the server advertises no maxWait (0)", async () => { + setServerOptions({ maxWait: 0 }); + const dif = makeDif("ok.so", "debug bytes", "id-ok"); + apiRequestToRegionMock.mockImplementationOnce( + async (_url: string, _endpoint: string, init: { body: object }) => { + const key = Object.keys(init.body as object)[0] as string; + return { data: { [key]: { state: "ok" } } }; + } + ); + + const results = await uploadDebugFiles({ + ...SOLE, + difs: [dif], + wait: true, + maxWaitMs: 60_000, + }); + + expect(results[0]?.state).toBe("ok"); + }); }); diff --git a/test/lib/dif/scan.test.ts b/test/lib/dif/scan.test.ts new file mode 100644 index 000000000..74d032ee1 --- /dev/null +++ b/test/lib/dif/scan.test.ts @@ -0,0 +1,107 @@ +/** + * Unit tests for the DIF scanner's I/O paths. + * + * Core filter invariants (format/id/feature matching, debug-id normalization) + * are covered by the property-based tests in scan.property.test.ts. These tests + * focus on filesystem behavior the property generators cannot express: the + * `prepareDifs` size gate. + */ + +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"; + +/** A minimal, valid Breakpad symbol file with a known debug id. */ +const BREAKPAD_FIXTURE = [ + "MODULE Linux x86_64 0F13A5DA412AFBF7C8662048F3294F3D0 example", + "INFO CODE_ID DAA5130F2A41F7FBC8662048F3294F3D439CA7FF", + "FUNC 1000 10 0 main", + "1000 10 42 1", + "PUBLIC 2000 0 some_symbol", +].join("\n"); + +let tempDir: string; + +beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "df-scan-test-")); +}); + +afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); +}); + +describe("prepareDifs size gate", () => { + test("keeps a file within the size limit", async () => { + const path = join(tempDir, "ok.sym"); + await writeFile(path, BREAKPAD_FIXTURE); + const files = await scanPaths([path]); + const { prepared, oversizedCount } = await prepareDifs( + files, + buildDifFilters({}), + { maxFileSize: 10 * 1024 } + ); + expect(prepared).toHaveLength(1); + expect(oversizedCount).toBe(0); + }); + + test("skips a file larger than the size limit and counts it", async () => { + const path = join(tempDir, "big.sym"); + await writeFile(path, BREAKPAD_FIXTURE); + const files = await scanPaths([path]); + // The fixture is well over 1 byte, so a 1-byte cap drops it. + const { prepared, oversizedCount } = await prepareDifs( + files, + buildDifFilters({}), + { maxFileSize: 1 } + ); + expect(prepared).toHaveLength(0); + expect(oversizedCount).toBe(1); + }); + + test("no size limit (0/omitted) keeps the file", async () => { + const path = join(tempDir, "nolimit.sym"); + await writeFile(path, BREAKPAD_FIXTURE); + const files = await scanPaths([path]); + const { prepared, oversizedCount } = await prepareDifs( + files, + buildDifFilters({}) + ); + expect(prepared).toHaveLength(1); + expect(oversizedCount).toBe(0); + }); + + test("does not count an oversized file of an unrequested --type", async () => { + const path = join(tempDir, "big.sym"); + await writeFile(path, BREAKPAD_FIXTURE); + const files = await scanPaths([path]); + // The fixture is a Breakpad file; with --type elf it is filtered out by the + // header format check before the size gate, so it must not be counted as + // oversized (which would otherwise misreport "all matched files too large"). + const { prepared, oversizedCount } = await prepareDifs( + files, + buildDifFilters({ types: ["elf"] }), + { maxFileSize: 1 } + ); + expect(prepared).toHaveLength(0); + expect(oversizedCount).toBe(0); + }); + + test("counts an oversized file that matches the requested --type", async () => { + const path = join(tempDir, "big.sym"); + await writeFile(path, BREAKPAD_FIXTURE); + const files = await scanPaths([path]); + const { prepared, oversizedCount } = await prepareDifs( + files, + buildDifFilters({ types: ["breakpad"] }), + { maxFileSize: 1 } + ); + expect(prepared).toHaveLength(0); + expect(oversizedCount).toBe(1); + }); +});