From 9167b1ffd37b967019ad36c1d4353d3283dd01d2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 6 Nov 2026 19:52:57 +0000 Subject: [PATCH 1/4] fix(cli): Batch objectstore requests in snapshots upload `sentry snapshots upload` sent one HEAD per image (8 at a time) and then one PUT per missing image. These go through Sentry's objectstore proxy (`/api/0/organizations/{org}/objectstore/`), which rate-limits each HTTP request. A suite of about 60 images got HTTP 429 on HEAD within about a second. The upload now uses objectstore's multipart batch endpoint (`/v1/objects:batch/{usecase}/{scope}/`), like the Rust client's `Session::many()` that sentry-cli uses. All HEADs go in one request. Files up to 1 MB are inserted in batches of up to 1000 operations and 100 MB. Larger files still get individual PUTs, 5 at a time. Co-Authored-By: Jamie Quadri --- packages/cli/src/commands/snapshots/upload.ts | 36 +- packages/cli/src/lib/objectstore.ts | 376 ++++++++++++++++-- .../test/commands/snapshots/upload.test.ts | 45 ++- packages/cli/test/lib/objectstore.test.ts | 179 +++++++-- 4 files changed, 535 insertions(+), 101 deletions(-) diff --git a/packages/cli/src/commands/snapshots/upload.ts b/packages/cli/src/commands/snapshots/upload.ts index f57fcfc89..ddead9f20 100644 --- a/packages/cli/src/commands/snapshots/upload.ts +++ b/packages/cli/src/commands/snapshots/upload.ts @@ -42,9 +42,9 @@ import { import { CommandOutput } from "../../lib/formatters/output.js"; import { logger } from "../../lib/logger.js"; import { + findExistingObjects, type ObjectstoreConfig, - objectExists, - putObject, + putObjects, } from "../../lib/objectstore.js"; import { resolveOrgAndProject } from "../../lib/resolve-target.js"; import { @@ -59,9 +59,6 @@ const log = logger.withTag("snapshots.upload"); const USAGE_HINT = "sentry snapshots upload --app-id "; -/** Concurrency for objectstore HEAD/PUT requests. */ -const UPLOAD_CONCURRENCY = 8; - /** Flags accepted by `snapshots upload`. */ type UploadFlags = { "app-id": string; @@ -204,17 +201,6 @@ async function resolveSelective( return { selective, allImageNames }; } -/** Run `fn` over `items` with bounded concurrency. */ -async function runPooled( - items: T[], - limit: number, - fn: (item: T) => Promise, -): Promise { - for (let i = 0; i < items.length; i += limit) { - await Promise.all(items.slice(i, i + limit).map(fn)); - } -} - /** A per-image manifest metadata entry (width/height override sidecar). */ function imageMetadata(image: CollectedImage): Record { return { @@ -276,17 +262,15 @@ async function uploadImages( log.warn(`Duplicate paths encountered, skipping: ${duplicates.join(", ")}`); } - // HEAD to find objects already present, then PUT only the missing ones. - const existing = new Set(); - await runPooled(prepared, UPLOAD_CONCURRENCY, async (item) => { - if (await objectExists(config, item.key)) { - existing.add(item.key); - } - }); + // Batched HEADs find objects already present, then only the missing ones are + // uploaded. Batching keeps the request count low enough to stay under the + // Sentry proxy's per-request rate limit. + const existing = await findExistingObjects( + config, + prepared.map((item) => item.key), + ); const missing = prepared.filter((item) => !existing.has(item.key)); - await runPooled(missing, UPLOAD_CONCURRENCY, async (item) => { - await putObject(config, item.key, await readFile(item.path)); - }); + await putObjects(config, missing); return { entries, diff --git a/packages/cli/src/lib/objectstore.ts b/packages/cli/src/lib/objectstore.ts index 2dce1c1eb..966747496 100644 --- a/packages/cli/src/lib/objectstore.ts +++ b/packages/cli/src/lib/objectstore.ts @@ -2,16 +2,23 @@ * Minimal Objectstore HTTP client for snapshot image uploads. * * Replicates the subset of the `objectstore-client` protocol (getsentry/objectstore) - * that `snapshots upload` needs: HEAD an object to check existence (dedup) and - * PUT its bytes. The service URL, scopes, auth token, and expiration policy all - * come from the Sentry `snapshots/upload-options/` endpoint — this client never - * signs tokens itself (the token is a pre-signed JWT). + * that `snapshots upload` needs: check which objects already exist (dedup) and + * upload the missing ones. The service URL, scopes, auth token, and expiration + * policy all come from the Sentry `snapshots/upload-options/` endpoint — this + * client never signs tokens itself (the token is a pre-signed JWT). * * Object path layout: `{serviceUrl}/v1/objects/{usecase}/{scope}/{key}` where * `scope` is a `;`-joined list of ordered `key=value` pairs. Auth is carried in * the `x-os-auth: Bearer ` header (not the standard `Authorization`). + * + * Like the Rust client's `Session::many()`, HEADs and small PUTs are grouped + * into multipart requests to `{serviceUrl}/v1/objects:batch/{usecase}/{scope}/`. + * The Sentry proxy in front of Objectstore rate-limits per HTTP request, so one + * request per image quickly trips it (HTTP 429) on larger snapshot suites. */ +import { randomUUID } from "node:crypto"; +import { readFile, stat } from "node:fs/promises"; import { customFetch } from "./custom-ca.js"; import { ApiError } from "./errors.js"; @@ -23,10 +30,30 @@ const EXPIRATION_HEADER = "x-sn-expiration"; /** Matches one or more trailing slashes (for base-URL normalization). */ const TRAILING_SLASHES = /\/+$/; -/** Timeout for a HEAD existence check. */ -const HEAD_TIMEOUT_MS = 30_000; /** Timeout for uploading a single object. */ const PUT_TIMEOUT_MS = 120_000; +/** Timeout for one batch request (up to {@link MAX_BATCH_BODY_SIZE} bytes). */ +const BATCH_TIMEOUT_MS = 300_000; + +/** Per-part headers of the batch protocol. */ +const BATCH_KIND_HEADER = "x-sn-batch-operation-kind"; +const BATCH_KEY_HEADER = "x-sn-batch-operation-key"; +const BATCH_INDEX_HEADER = "x-sn-batch-operation-index"; +const BATCH_STATUS_HEADER = "x-sn-batch-operation-status"; + +/** Server-enforced maximum number of operations per batch request. */ +const MAX_BATCH_OPS = 1000; +/** Server-enforced maximum body size of one batch part; larger objects are PUT individually. */ +const MAX_BATCH_PART_SIZE = 1024 * 1024; +/** Maximum total body size of one batch request (matches the Rust client). */ +const MAX_BATCH_BODY_SIZE = 100 * 1024 * 1024; +/** Concurrency for objects too large to batch (matches the Rust client). */ +const INDIVIDUAL_CONCURRENCY = 5; + +/** Matches the `boundary` parameter of a multipart `Content-Type`. */ +const BOUNDARY_PARAM = /boundary=(?:"([^"]+)"|([^;\s]+))/i; +/** Matches the CRLF that terminates a multipart part body. */ +const TRAILING_CRLF = /\r\n$/; /** * Objectstore upload configuration, as returned (camelCase) by the Sentry @@ -62,6 +89,30 @@ export function buildObjectUrl(config: ObjectstoreConfig, key: string): string { )}/${key}`; } +/** Build the batch endpoint URL for the configured usecase + scope. */ +function buildBatchUrl(config: ObjectstoreConfig): string { + const base = config.url.replace(TRAILING_SLASHES, ""); + return `${base}/v1/objects:batch/${config.usecase}/${scopeSegment( + config.scopes, + )}/`; +} + +/** + * Escape a value for an Objectstore header, matching `encode_header_value` in + * `objectstore-types`: percent-encode control bytes, DEL, `%`, and non-ASCII. + */ +export function encodeHeaderValue(value: string): string { + let out = ""; + for (const byte of Buffer.from(value, "utf8")) { + if (byte < 0x20 || byte >= 0x7f || byte === 0x25) { + out += `%${byte.toString(16).toUpperCase().padStart(2, "0")}`; + } else { + out += String.fromCharCode(byte); + } + } + return out; +} + /** Auth headers for an objectstore request, if a token is configured. */ function authHeaders(config: ObjectstoreConfig): Record { return config.authToken @@ -70,65 +121,318 @@ function authHeaders(config: ObjectstoreConfig): Record { } /** - * Check whether an object already exists (used to skip re-uploads). + * Upload an object's bytes (raw — no content encoding). + * + * The key is derived from the original file's SHA-256, so dedup via + * {@link findExistingObjects} is independent of any upload compression; images are + * already-compressed formats, so storing them raw avoids pointless CPU. * - * @returns `true` if the object exists, `false` on a 404. - * @throws {ApiError} On any non-2xx, non-404 response. + * @throws {ApiError} On a non-2xx response. */ -export async function objectExists( +export async function putObject( config: ObjectstoreConfig, key: string, -): Promise { + body: Uint8Array, +): Promise { const url = buildObjectUrl(config, key); const response = await customFetch(url, { - method: "HEAD", - headers: authHeaders(config), - signal: AbortSignal.timeout(HEAD_TIMEOUT_MS), + method: "PUT", + headers: { + ...authHeaders(config), + [EXPIRATION_HEADER]: config.expirationPolicy, + }, + body, + signal: AbortSignal.timeout(PUT_TIMEOUT_MS), }); - if (response.status === 404) { - return false; - } if (!response.ok) { throw new ApiError( - "Objectstore HEAD failed", + "Objectstore upload failed", response.status, - response.statusText || "HEAD failed", + response.statusText || "PUT failed", url, ); } - return true; +} + +/** One operation within a batch request. */ +type BatchOperation = + | { kind: "head"; key: string } + | { kind: "insert"; key: string; body: Uint8Array }; + +/** Outcome of one batch operation, as reported in its response part. */ +type BatchOperationResult = { + /** HTTP status of this operation (e.g. 200, 201, 404, 429). */ + status: number; + /** Response part body (an error description for failed operations). */ + message: string; +}; + +/** Serialize batch operations into a `multipart/form-data` request body. */ +function encodeBatchBody( + config: ObjectstoreConfig, + operations: BatchOperation[], + boundary: string, +): Buffer { + const chunks: Uint8Array[] = []; + for (const op of operations) { + let head = + `--${boundary}\r\n` + + 'content-disposition: form-data; name="part"\r\n' + + `${BATCH_KIND_HEADER}: ${op.kind}\r\n` + + `${BATCH_KEY_HEADER}: ${encodeHeaderValue(op.key)}\r\n`; + if (op.kind === "insert") { + head += `${EXPIRATION_HEADER}: ${config.expirationPolicy}\r\n`; + } + chunks.push(Buffer.from(`${head}\r\n`)); + if (op.kind === "insert") { + chunks.push(op.body); + } + chunks.push(Buffer.from("\r\n")); + } + chunks.push(Buffer.from(`--${boundary}--\r\n`)); + return Buffer.concat(chunks); +} + +/** Parse the header block of one multipart part into a lowercase-keyed map. */ +function parsePartHeaders(block: string): Map { + const headers = new Map(); + for (const line of block.split("\r\n")) { + const colon = line.indexOf(":"); + if (colon > 0) { + headers.set( + line.slice(0, colon).trim().toLowerCase(), + line.slice(colon + 1).trim(), + ); + } + } + return headers; } /** - * Upload an object's bytes (raw — no content encoding). + * Parse a batch `multipart/form-data` response into per-operation results, + * keyed by the operation index the server echoes on each part. * - * The key is derived from the original file's SHA-256, so dedup via - * {@link objectExists} is independent of any upload compression; images are - * already-compressed formats, so storing them raw avoids pointless CPU. + * Response parts only carry status headers and small error bodies, so the + * whole response is buffered and split on the boundary. + */ +function parseBatchResponse( + contentType: string, + body: Buffer, + url: string, +): Map { + const match = BOUNDARY_PARAM.exec(contentType); + const boundary = match?.[1] ?? match?.[2]; + if (!boundary) { + throw new ApiError( + "Objectstore batch response is not multipart", + 502, + `Unexpected Content-Type: ${contentType || "(none)"}`, + url, + ); + } + + const results = new Map(); + // latin1 maps bytes 1:1, so offsets stay valid for binary-safe splitting. + const segments = body.toString("latin1").split(`--${boundary}`); + for (const segment of segments.slice(1)) { + if (segment.startsWith("--")) { + break; // closing delimiter + } + const part = segment.startsWith("\r\n") ? segment.slice(2) : segment; + const split = part.indexOf("\r\n\r\n"); + if (split < 0) { + continue; + } + const headers = parsePartHeaders(part.slice(0, split)); + const index = Number.parseInt(headers.get(BATCH_INDEX_HEADER) ?? "", 10); + const status = Number.parseInt(headers.get(BATCH_STATUS_HEADER) ?? "", 10); + if (Number.isNaN(index) || Number.isNaN(status)) { + continue; + } + const rawBody = part.slice(split + 4).replace(TRAILING_CRLF, ""); + results.set(index, { + status, + message: Buffer.from(rawBody, "latin1").toString("utf8"), + }); + } + return results; +} + +/** + * Send operations as one batch request. * - * @throws {ApiError} On a non-2xx response. + * @returns Results aligned with `operations` by index. + * @throws {ApiError} When the batch request itself fails or the response is + * missing a result for any operation. */ -export async function putObject( +async function sendBatch( config: ObjectstoreConfig, - key: string, - body: Uint8Array, -): Promise { - const url = buildObjectUrl(config, key); + operations: BatchOperation[], +): Promise { + const url = buildBatchUrl(config); + const boundary = `sentry-cli-${randomUUID()}`; const response = await customFetch(url, { - method: "PUT", + method: "POST", headers: { ...authHeaders(config), - [EXPIRATION_HEADER]: config.expirationPolicy, + "content-type": `multipart/form-data; boundary="${boundary}"`, }, - body, - signal: AbortSignal.timeout(PUT_TIMEOUT_MS), + body: encodeBatchBody(config, operations, boundary), + signal: AbortSignal.timeout(BATCH_TIMEOUT_MS), }); if (!response.ok) { throw new ApiError( - "Objectstore upload failed", + "Objectstore batch request failed", response.status, - response.statusText || "PUT failed", + response.statusText || "batch request failed", url, ); } + + const parsed = parseBatchResponse( + response.headers.get("content-type") ?? "", + Buffer.from(await response.arrayBuffer()), + url, + ); + return operations.map((op, index) => { + const result = parsed.get(index); + if (!result) { + throw new ApiError( + "Objectstore batch response is incomplete", + 502, + `No result for ${op.kind} of ${op.key}`, + url, + ); + } + return result; + }); +} + +/** Throw an {@link ApiError} for a failed operation inside a batch. */ +function throwOperationError( + config: ObjectstoreConfig, + message: string, + key: string, + result: BatchOperationResult, +): never { + throw new ApiError( + message, + result.status, + result.message || `HTTP ${result.status}`, + buildObjectUrl(config, key), + ); +} + +/** Split `items` into consecutive chunks of at most `size` items. */ +function chunk(items: T[], size: number): T[][] { + const chunks: T[][] = []; + for (let i = 0; i < items.length; i += size) { + chunks.push(items.slice(i, i + size)); + } + return chunks; +} + +/** + * Find which keys already exist, using batched HEAD operations (one request + * per {@link MAX_BATCH_OPS} keys). + * + * @returns The subset of `keys` that exist. + * @throws {ApiError} On a failed batch, or any HEAD result other than 2xx/404. + */ +export async function findExistingObjects( + config: ObjectstoreConfig, + keys: string[], +): Promise> { + const existing = new Set(); + for (const batch of chunk(keys, MAX_BATCH_OPS)) { + const results = await sendBatch( + config, + batch.map((key) => ({ kind: "head", key })), + ); + results.forEach((result, i) => { + const key = batch[i] as string; + if (result.status === 404) { + return; + } + if (result.status >= 400) { + throwOperationError(config, "Objectstore HEAD failed", key, result); + } + existing.add(key); + }); + } + return existing; +} + +/** A file to upload under an object key. */ +export type ObjectUpload = { key: string; path: string }; + +/** Group sized uploads into batches within the op-count and body-size limits. */ +function groupBatches( + uploads: (ObjectUpload & { size: number })[], +): ObjectUpload[][] { + const batches: ObjectUpload[][] = []; + let current: ObjectUpload[] = []; + let currentSize = 0; + for (const upload of uploads) { + if ( + current.length >= MAX_BATCH_OPS || + (current.length > 0 && currentSize + upload.size > MAX_BATCH_BODY_SIZE) + ) { + batches.push(current); + current = []; + currentSize = 0; + } + current.push(upload); + currentSize += upload.size; + } + if (current.length > 0) { + batches.push(current); + } + return batches; +} + +/** + * Upload files as objects. Files up to {@link MAX_BATCH_PART_SIZE} are sent in + * batch requests (one at a time, to bound memory); larger files are PUT + * individually with bounded concurrency. + * + * @throws {ApiError} On a failed batch, or any failed insert. + */ +export async function putObjects( + config: ObjectstoreConfig, + uploads: ObjectUpload[], +): Promise { + const sized = await Promise.all( + uploads.map(async (upload) => ({ + ...upload, + size: (await stat(upload.path)).size, + })), + ); + const batchable = sized.filter((u) => u.size <= MAX_BATCH_PART_SIZE); + const individual = sized.filter((u) => u.size > MAX_BATCH_PART_SIZE); + + for (const batch of groupBatches(batchable)) { + const operations = await Promise.all( + batch.map(async (upload): Promise => ({ + kind: "insert", + key: upload.key, + body: await readFile(upload.path), + })), + ); + const results = await sendBatch(config, operations); + results.forEach((result, i) => { + if (result.status >= 400) { + const key = (batch[i] as ObjectUpload).key; + throwOperationError(config, "Objectstore upload failed", key, result); + } + }); + } + + for (const group of chunk(individual, INDIVIDUAL_CONCURRENCY)) { + await Promise.all( + group.map(async (upload) => { + await putObject(config, upload.key, await readFile(upload.path)); + }), + ); + } } diff --git a/packages/cli/test/commands/snapshots/upload.test.ts b/packages/cli/test/commands/snapshots/upload.test.ts index e44112377..f003d4d84 100644 --- a/packages/cli/test/commands/snapshots/upload.test.ts +++ b/packages/cli/test/commands/snapshots/upload.test.ts @@ -80,8 +80,10 @@ describe("snapshots upload", () => { uploadOptionsSpy = vi .spyOn(preprod, "fetchSnapshotsUploadOptions") .mockResolvedValue(UPLOAD_OPTIONS); - existsSpy = vi.spyOn(objectstore, "objectExists").mockResolvedValue(false); - putSpy = vi.spyOn(objectstore, "putObject").mockResolvedValue(undefined); + existsSpy = vi + .spyOn(objectstore, "findExistingObjects") + .mockResolvedValue(new Set()); + putSpy = vi.spyOn(objectstore, "putObjects").mockResolvedValue(undefined); createSpy = vi.spyOn(preprod, "createPreprodSnapshot").mockResolvedValue({ artifactId: "snap-1", imageCount: 2, @@ -94,6 +96,13 @@ describe("snapshots upload", () => { await rm(tmpDir, { recursive: true, force: true }); }); + /** Object keys passed to `putObjects` across all calls. */ + function uploadedKeys(): string[] { + return putSpy.mock.calls.flatMap(([, uploads]) => + (uploads as objectstore.ObjectUpload[]).map((u) => u.key), + ); + } + /** Write a folder with two PNGs (one nested) + a sidecar; returns its path. */ async function writeShots(): Promise { const dir = join(tmpDir, "shots"); @@ -115,8 +124,9 @@ describe("snapshots upload", () => { await func.call(harness.context, { "app-id": "com.example.app" }, dir); - // Two images uploaded (none pre-existing). - expect(putSpy).toHaveBeenCalledTimes(2); + // Two images uploaded in one call (none pre-existing). + expect(putSpy).toHaveBeenCalledTimes(1); + expect(uploadedKeys()).toHaveLength(2); expect(createSpy).toHaveBeenCalledTimes(1); const [, , manifest] = createSpy.mock.calls[0] as [ @@ -139,13 +149,19 @@ describe("snapshots upload", () => { // The objectstore key is `{orgId}/{projectId}/{sha256}` from the scope. const hash = images["a.png"].content_hash as string; - const key = putSpy.mock.calls.find(([, k]) => - (k as string).endsWith(hash), - )?.[1] as string; + const key = uploadedKeys().find((k) => k.endsWith(hash)) as string; expect(key).toMatch(/^1\/2\/[0-9a-f]{64}$/); - expect(key.endsWith(hash)).toBe(true); - expect(existsSpy).toHaveBeenCalledWith(config, key); - expect(putSpy).toHaveBeenCalledWith(config, key, expect.any(Uint8Array)); + expect(existsSpy).toHaveBeenCalledTimes(1); + expect(existsSpy).toHaveBeenCalledWith( + config, + expect.arrayContaining([key]), + ); + expect(putSpy).toHaveBeenCalledWith( + config, + expect.arrayContaining([ + { key, path: expect.stringMatching(/a\.png$/) }, + ]), + ); }, ); @@ -200,20 +216,23 @@ describe("snapshots upload", () => { await func.call(harness.context, { "app-id": "app" }, "shots"); - expect(putSpy).toHaveBeenCalledTimes(2); + expect(uploadedKeys()).toHaveLength(2); expect(createSpy).toHaveBeenCalledTimes(1); }); test("skips objects already present in objectstore", async () => { const dir = await writeShots(); // First image already exists, second does not. - existsSpy.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + existsSpy.mockImplementation( + async (_config: unknown, requested: string[]) => new Set([requested[0]]), + ); const harness = createContext(); const func = await uploadCommand.loader(); await func.call(harness.context, { "app-id": "app" }, dir); - expect(putSpy).toHaveBeenCalledTimes(1); + const [, keys] = existsSpy.mock.calls[0] as [unknown, string[]]; + expect(uploadedKeys()).toEqual([keys[1]]); }); test("reports no images and creates nothing for an empty folder", async () => { diff --git a/packages/cli/test/lib/objectstore.test.ts b/packages/cli/test/lib/objectstore.test.ts index 71cfb48a2..da8e29355 100644 --- a/packages/cli/test/lib/objectstore.test.ts +++ b/packages/cli/test/lib/objectstore.test.ts @@ -1,10 +1,14 @@ /** * Tests for the minimal Objectstore HTTP client. * - * `customFetch` is mocked so URL construction, the `x-os-auth` header, HEAD - * existence semantics, and PUT request shape can be verified without a network. + * `customFetch` is mocked so URL construction, the `x-os-auth` header, the + * batch request/response protocol, and PUT request shape can be verified + * without a network. */ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { ApiError } from "../../src/lib/errors.js"; @@ -13,9 +17,11 @@ vi.mock("../../src/lib/custom-ca.js", () => ({ customFetch: customFetchMock })); import { buildObjectUrl, + encodeHeaderValue, + findExistingObjects, type ObjectstoreConfig, - objectExists, putObject, + putObjects, } from "../../src/lib/objectstore.js"; const config: ObjectstoreConfig = { @@ -46,38 +52,159 @@ describe("buildObjectUrl", () => { }); }); -describe("objectExists", () => { - test("returns true on a 2xx HEAD and sends the auth header", async () => { - customFetchMock.mockResolvedValue({ ok: true, status: 200 }); - expect(await objectExists(config, "123/456/abc")).toBe(true); +/** Build a batch response in the server's `multipart/form-data` shape. */ +function batchResponse( + parts: { index: number; status: string; body?: string }[], +): Response { + const boundary = "os-boundary-00000000000000000000000000000abc"; + let text = ""; + for (const part of parts) { + text += + `--${boundary}\r\n` + + `x-sn-batch-operation-index: ${part.index}\r\n` + + `x-sn-batch-operation-status: ${part.status}\r\n` + + "content-disposition: form-data; name=part\r\n\r\n" + + `${part.body ?? ""}\r\n`; + } + text += `--${boundary}--`; + return new Response(text, { + status: 200, + headers: { "content-type": `multipart/form-data; boundary="${boundary}"` }, + }); +} + +/** Decode a captured batch request body as multipart form data. */ +async function sentParts(init: RequestInit): Promise { + const headers = init.headers as Record; + const form = await new Response(init.body as BodyInit, { + headers: { "content-type": headers["content-type"] as string }, + }).formData(); + return form.getAll("part"); +} + +describe("encodeHeaderValue", () => { + test("leaves visible ASCII alone and escapes %, controls, and non-ASCII", () => { + expect(encodeHeaderValue("1/2/abc")).toBe("1/2/abc"); + expect(encodeHeaderValue("100% done")).toBe("100%25 done"); + expect(encodeHeaderValue("r\u00e9port")).toBe("r%C3%A9port"); + expect(encodeHeaderValue("a\nb")).toBe("a%0Ab"); + }); +}); + +describe("findExistingObjects", () => { + test("sends all HEADs in one batch request and returns existing keys", async () => { + customFetchMock.mockResolvedValue( + batchResponse([ + { index: 1, status: "404 Not Found" }, + { index: 0, status: "200 OK" }, + { index: 2, status: "200 OK" }, + ]), + ); + + const existing = await findExistingObjects(config, ["k0", "k1", "k2"]); + + expect([...existing].sort()).toEqual(["k0", "k2"]); + expect(customFetchMock).toHaveBeenCalledTimes(1); const [url, init] = customFetchMock.mock.calls[0] ?? []; - expect(url).toContain( - "/v1/objects/preprod_snapshots/org=123;project=456/123/456/abc", + expect(url).toBe( + "https://objectstore.example.com/v1/objects:batch/preprod_snapshots/org=123;project=456/", ); - expect(init.method).toBe("HEAD"); + expect(init.method).toBe("POST"); expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); - expect(init.signal).toBeInstanceOf(AbortSignal); + expect(await sentParts(init)).toHaveLength(3); + const raw = Buffer.from(init.body).toString(); + expect(raw).toContain("x-sn-batch-operation-kind: head\r\n"); + expect(raw).toContain("x-sn-batch-operation-key: k1\r\n"); }); - test("returns false on a 404", async () => { - customFetchMock.mockResolvedValue({ ok: false, status: 404 }); - expect(await objectExists(config, "123/456/abc")).toBe(false); + test("throws on a per-operation error other than 404", async () => { + customFetchMock.mockResolvedValue( + batchResponse([ + { index: 0, status: "200 OK" }, + { index: 1, status: "429 Too Many Requests", body: "slow down" }, + ]), + ); + await expect(findExistingObjects(config, ["k0", "k1"])).rejects.toThrow( + ApiError, + ); }); - test("throws on other non-2xx responses", async () => { - customFetchMock.mockResolvedValue({ - ok: false, - status: 500, - statusText: "err", - }); - await expect(objectExists(config, "123/456/abc")).rejects.toThrow(ApiError); + test("throws when the batch request itself fails", async () => { + customFetchMock.mockResolvedValue( + new Response(null, { status: 429, statusText: "Too Many Requests" }), + ); + const error = await findExistingObjects(config, ["k0"]).catch((e) => e); + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(429); }); - test("omits the auth header when no token is configured", async () => { - customFetchMock.mockResolvedValue({ ok: true, status: 200 }); - await objectExists({ ...config, authToken: null }, "k"); - const [, init] = customFetchMock.mock.calls[0] ?? []; - expect(init.headers["x-os-auth"]).toBeUndefined(); + test("throws when the response is missing an operation", async () => { + customFetchMock.mockResolvedValue( + batchResponse([{ index: 0, status: "200 OK" }]), + ); + await expect(findExistingObjects(config, ["k0", "k1"])).rejects.toThrow( + ApiError, + ); + }); +}); + +describe("putObjects", () => { + let dir: string; + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "objectstore-")); + }); + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + test("batches small files and PUTs files over the batch part limit", async () => { + const small = join(dir, "small.png"); + const large = join(dir, "large.png"); + await writeFile(small, new Uint8Array([1, 2, 3])); + await writeFile(large, new Uint8Array(1024 * 1024 + 1)); + customFetchMock.mockImplementation(async (_url, init: RequestInit) => + init.method === "POST" + ? batchResponse([{ index: 0, status: "201 Created" }]) + : { ok: true, status: 201 }, + ); + + await putObjects(config, [ + { key: "1/2/small", path: small }, + { key: "1/2/large", path: large }, + ]); + + expect(customFetchMock).toHaveBeenCalledTimes(2); + const [, batchInit] = customFetchMock.mock.calls.find( + ([, init]) => init.method === "POST", + ) ?? [undefined, undefined]; + const parts = await sentParts(batchInit); + expect(parts).toHaveLength(1); + // Parts without a filename decode as strings. + expect(parts[0]).toBe("\x01\x02\x03"); + const raw = Buffer.from(batchInit.body).toString("latin1"); + expect(raw).toContain("x-sn-batch-operation-kind: insert\r\n"); + expect(raw).toContain("x-sn-batch-operation-key: 1/2/small\r\n"); + expect(raw).toContain("x-sn-expiration: ttl:30d\r\n"); + + const [putUrl, putInit] = customFetchMock.mock.calls.find( + ([, init]) => init.method === "PUT", + ) ?? [undefined, undefined]; + expect(putUrl).toMatch( + /\/v1\/objects\/preprod_snapshots\/.*\/1\/2\/large$/, + ); + expect(putInit.body.byteLength).toBe(1024 * 1024 + 1); + }); + + test("throws when an insert in the batch fails", async () => { + const small = join(dir, "small.png"); + await writeFile(small, new Uint8Array([1])); + customFetchMock.mockResolvedValue( + batchResponse([{ index: 0, status: "500 Internal Server Error" }]), + ); + await expect( + putObjects(config, [{ key: "k", path: small }]), + ).rejects.toThrow(ApiError); }); }); From 5459297d55423590122a01f245cd09a7aceaaa14 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:41:33 +0000 Subject: [PATCH 2/4] fix(cli): Send objectstore batches concurrently Runs up to 4 batch requests at once for both existence checks and uploads, and uploads oversized files in parallel with the batches. Lowers the per-batch body limit to 25 MB so in-flight upload bodies stay around 100 MB, the same bound as before. --- packages/cli/src/lib/objectstore.ts | 109 +++++++++++++--------- packages/cli/test/lib/objectstore.test.ts | 25 +++++ 2 files changed, 91 insertions(+), 43 deletions(-) diff --git a/packages/cli/src/lib/objectstore.ts b/packages/cli/src/lib/objectstore.ts index 966747496..813d1dc30 100644 --- a/packages/cli/src/lib/objectstore.ts +++ b/packages/cli/src/lib/objectstore.ts @@ -19,6 +19,7 @@ import { randomUUID } from "node:crypto"; import { readFile, stat } from "node:fs/promises"; +import pLimit from "p-limit"; import { customFetch } from "./custom-ca.js"; import { ApiError } from "./errors.js"; @@ -45,8 +46,16 @@ const BATCH_STATUS_HEADER = "x-sn-batch-operation-status"; const MAX_BATCH_OPS = 1000; /** Server-enforced maximum body size of one batch part; larger objects are PUT individually. */ const MAX_BATCH_PART_SIZE = 1024 * 1024; -/** Maximum total body size of one batch request (matches the Rust client). */ -const MAX_BATCH_BODY_SIZE = 100 * 1024 * 1024; +/** + * Maximum total body size of one batch request. With {@link BATCH_CONCURRENCY} + * batches in flight, upload bodies held in memory stay around 100 MB. + */ +const MAX_BATCH_BODY_SIZE = 25 * 1024 * 1024; +/** + * Batch requests in flight at once. Each batch is one request to the Sentry + * proxy, so this stays far below its per-request rate limit. + */ +const BATCH_CONCURRENCY = 4; /** Concurrency for objects too large to batch (matches the Rust client). */ const INDIVIDUAL_CONCURRENCY = 5; @@ -334,7 +343,7 @@ function chunk(items: T[], size: number): T[][] { /** * Find which keys already exist, using batched HEAD operations (one request - * per {@link MAX_BATCH_OPS} keys). + * per {@link MAX_BATCH_OPS} keys, up to {@link BATCH_CONCURRENCY} at once). * * @returns The subset of `keys` that exist. * @throws {ApiError} On a failed batch, or any HEAD result other than 2xx/404. @@ -344,22 +353,27 @@ export async function findExistingObjects( keys: string[], ): Promise> { const existing = new Set(); - for (const batch of chunk(keys, MAX_BATCH_OPS)) { - const results = await sendBatch( - config, - batch.map((key) => ({ kind: "head", key })), - ); - results.forEach((result, i) => { - const key = batch[i] as string; - if (result.status === 404) { - return; - } - if (result.status >= 400) { - throwOperationError(config, "Objectstore HEAD failed", key, result); - } - existing.add(key); - }); - } + const limit = pLimit(BATCH_CONCURRENCY); + await Promise.all( + chunk(keys, MAX_BATCH_OPS).map((batch) => + limit(async () => { + const results = await sendBatch( + config, + batch.map((key) => ({ kind: "head", key })), + ); + results.forEach((result, i) => { + const key = batch[i] as string; + if (result.status === 404) { + return; + } + if (result.status >= 400) { + throwOperationError(config, "Objectstore HEAD failed", key, result); + } + existing.add(key); + }); + }), + ), + ); return existing; } @@ -393,8 +407,9 @@ function groupBatches( /** * Upload files as objects. Files up to {@link MAX_BATCH_PART_SIZE} are sent in - * batch requests (one at a time, to bound memory); larger files are PUT - * individually with bounded concurrency. + * batch requests, up to {@link BATCH_CONCURRENCY} at once; larger files are PUT + * individually, up to {@link INDIVIDUAL_CONCURRENCY} at once. Both run in + * parallel. A file is read only when its request starts. * * @throws {ApiError} On a failed batch, or any failed insert. */ @@ -411,28 +426,36 @@ export async function putObjects( const batchable = sized.filter((u) => u.size <= MAX_BATCH_PART_SIZE); const individual = sized.filter((u) => u.size > MAX_BATCH_PART_SIZE); - for (const batch of groupBatches(batchable)) { - const operations = await Promise.all( - batch.map(async (upload): Promise => ({ - kind: "insert", - key: upload.key, - body: await readFile(upload.path), - })), - ); - const results = await sendBatch(config, operations); - results.forEach((result, i) => { - if (result.status >= 400) { - const key = (batch[i] as ObjectUpload).key; - throwOperationError(config, "Objectstore upload failed", key, result); - } - }); - } - - for (const group of chunk(individual, INDIVIDUAL_CONCURRENCY)) { - await Promise.all( - group.map(async (upload) => { + const batchLimit = pLimit(BATCH_CONCURRENCY); + const individualLimit = pLimit(INDIVIDUAL_CONCURRENCY); + await Promise.all([ + ...groupBatches(batchable).map((batch) => + batchLimit(async () => { + const operations = await Promise.all( + batch.map(async (upload): Promise => ({ + kind: "insert", + key: upload.key, + body: await readFile(upload.path), + })), + ); + const results = await sendBatch(config, operations); + results.forEach((result, i) => { + if (result.status >= 400) { + const key = (batch[i] as ObjectUpload).key; + throwOperationError( + config, + "Objectstore upload failed", + key, + result, + ); + } + }); + }), + ), + ...individual.map((upload) => + individualLimit(async () => { await putObject(config, upload.key, await readFile(upload.path)); }), - ); - } + ), + ]); } diff --git a/packages/cli/test/lib/objectstore.test.ts b/packages/cli/test/lib/objectstore.test.ts index da8e29355..6587ea9a9 100644 --- a/packages/cli/test/lib/objectstore.test.ts +++ b/packages/cli/test/lib/objectstore.test.ts @@ -117,6 +117,31 @@ describe("findExistingObjects", () => { expect(raw).toContain("x-sn-batch-operation-key: k1\r\n"); }); + test("splits more than 1000 keys into batches sent concurrently", async () => { + let inFlight = 0; + let maxInFlight = 0; + customFetchMock.mockImplementation(async (_url, init: RequestInit) => { + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + const count = (await sentParts(init)).length; + await new Promise((resolve) => setTimeout(resolve, 10)); + inFlight -= 1; + return batchResponse( + Array.from({ length: count }, (_, index) => ({ + index, + status: "200 OK", + })), + ); + }); + const keys = Array.from({ length: 2500 }, (_, i) => `k${i}`); + + const existing = await findExistingObjects(config, keys); + + expect(existing.size).toBe(2500); + expect(customFetchMock).toHaveBeenCalledTimes(3); + expect(maxInFlight).toBe(3); + }); + test("throws on a per-operation error other than 404", async () => { customFetchMock.mockResolvedValue( batchResponse([ From 3e5a795d23dc0a4b8538ebb1c46f625376b2216e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 6 Nov 2026 23:13:33 +0000 Subject: [PATCH 3/4] docs: Make Oxlint the default for new lint rules New lint rules go in .oxlintrc.json or the CLI Oxlint JS plugin, not ast-grep. Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com> --- AGENTS.md | 1 + docs/contributing/coding-guidelines.md | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index de0c60203..3e27c63a5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,7 @@ Toolkit contains the Sentry CLI, the Sentry MCP server, and shared code. Product - **MCP tool count**: `EXPOSE_ALL_TOOLS` lists every tool and relies on harness tool search. The gateway surface it falls back to (`TOP_LEVEL_TOOL_NAMES`) targets ≤20, hard limit 25. - **Quality gate**: `pnpm run tsc && pnpm run lint && pnpm run test` must pass before committing. - **Dependency direction**: `sentry` (cli) → `@sentry/mcp-core` → `@sentry/toolkit-core`, and `@sentry/mcp-cloudflare` → `@sentry/mcp-core`; never the reverse. mcp-core and toolkit-core never import the CLI. Inject CLI-only capabilities (SQLite cache, local-file DSN detection, file uploads) through interfaces defined in mcp-core or toolkit-core. `pnpm run lint` enforces this; see docs/architecture/overview.md#package-dependency-direction. +- **Lint rules**: Write new lint rules for Oxlint: a built-in rule in `.oxlintrc.json`, or a custom rule in `packages/cli/lint-rules/cli-oxlint-plugin.js`. Do not add ast-grep rules. See docs/contributing/coding-guidelines.md#formatting-and-linting. - **Package CI**: Put checks for one workspace package in its `package.json` scripts and `sentryCi`, which `test.yml` runs per project. Do not add a workflow for them. See docs/operations/github-actions.md#package-ci-checks. ## Repository Structure diff --git a/docs/contributing/coding-guidelines.md b/docs/contributing/coding-guidelines.md index f0f5f40ab..e34d61b3c 100644 --- a/docs/contributing/coding-guidelines.md +++ b/docs/contributing/coding-guidelines.md @@ -30,6 +30,10 @@ product-specific implementation patterns. - Root, MCP, and CLI files use Oxlint and Oxfmt. The CLI-specific rules live in `packages/cli/lint-rules/cli-oxlint-plugin.js`, with inline Oxlint suppressions for intentional exceptions. +- Write new lint rules for Oxlint: turn on a built-in rule in `.oxlintrc.json` + (for example `no-restricted-imports`), or add a custom rule to the Oxlint JS + plugin above and register it in `.oxlintrc.json`. Do not add new ast-grep + rules; the few left in `ast-grep-rules/` predate this. - Oxlint runs type-aware (`options.typeAware` in `.oxlintrc.json`, backed by `oxlint-tsgolint`). `typescript/return-await` (`always`) is on repo-wide: write `return await` in async functions so callers stay in async stack From a5f4abd6c885a2f8277a9b6e01559ba27067c313 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 7 Nov 2026 00:16:44 +0000 Subject: [PATCH 4/4] ref(cli): Use the @sentry/api Objectstore client for snapshot uploads Bumps @sentry/api to 1.44.0, which ships the batched Objectstore client from getsentry/sentry-api-schema#133, and replaces the CLI's own protocol code with it. src/lib/objectstore.ts is now a thin adapter: it reads files from disk when their request starts, sends requests through customFetch so custom CAs still apply, and rethrows ObjectstoreError as the CLI's ApiError. Batching and limits are unchanged: 4 concurrent batches, 25 MB per batch, and 5 concurrent PUTs for files over 1 MB. Protocol tests now live in @sentry/api; the CLI tests cover the adapter. The bump also regenerates the alert and event skill references, which pick up field descriptions from the newer schema. Co-Authored-By: Jamie Quadri Co-Authored-By: Burak Yigit Kaya --- .../skills/sentry-cli/references/alert.md | 50 +- .../skills/sentry-cli/references/event.md | 72 +-- packages/cli/src/lib/objectstore.ts | 464 ++---------------- packages/cli/test/lib/objectstore.test.ts | 152 ++---- pnpm-lock.yaml | 25 +- pnpm-workspace.yaml | 4 +- 6 files changed, 160 insertions(+), 607 deletions(-) diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md index a24671da9..dd12d2bfc 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md @@ -190,18 +190,18 @@ Edit an issue alert rule |-------|------|-------------| | `id` | string | Alert rule ID | | `name` | string | Alert rule name | -| `organizationId` | string | | -| `createdBy` | string \| null | | -| `dateCreated` | string | | -| `dateUpdated` | string | | -| `triggers` | object \| null | | -| `actionFilters` | array \| null | | -| `environment` | string \| null | | -| `config` | object | | -| `detectorIds` | array \| null | | -| `enabled` | boolean | | -| `lastTriggered` | string \| null | | -| `owner` | string \| null | | +| `organizationId` | string | ID of the organization the alert belongs to | +| `createdBy` | string \| null | ID of the user who created the alert | +| `dateCreated` | string | When the alert was created (ISO 8601) | +| `dateUpdated` | string | When the alert was last updated (ISO 8601) | +| `triggers` | object \| null | Conditions on which the alert triggers | +| `actionFilters` | array \| null | Filters to run before actions fire, each with the actions to fire | +| `environment` | string \| null | Name of the environment the alert is limited to. Null for all environments | +| `config` | object | Configuration of the alert, such as frequency in minutes | +| `detectorIds` | array \| null | IDs of the monitors connected to the alert | +| `enabled` | boolean | Whether the alert is enabled | +| `lastTriggered` | string \| null | When the alert last fired (ISO 8601) | +| `owner` | string \| null | Team or user that owns the alert, such as user:123 or team:456 | | `org` | string | Organization slug | | `project` | string | Project slug | | `status` | string | Rule status (active, disabled) | @@ -389,21 +389,21 @@ Edit a metric alert rule | Field | Type | Description | |-------|------|-------------| -| `owner` | object \| null | | -| `createdBy` | string \| null | | -| `latestGroup` | object \| null | | -| `description` | string \| null | | +| `owner` | object \| null | Team or user that owns the monitor | +| `createdBy` | string \| null | ID of the user who created the monitor | +| `latestGroup` | object \| null | Latest issue the monitor created | +| `description` | string \| null | Description of the monitor | | `id` | string | Alert rule ID | -| `projectId` | string \| null | | +| `projectId` | string \| null | ID of the project the monitor belongs to | | `name` | string | Alert rule name | -| `type` | string | | -| `workflowIds` | array \| null | | -| `dateCreated` | string | | -| `dateUpdated` | string | | -| `dataSources` | array \| null | | -| `conditionGroup` | object \| null | | -| `config` | object | | -| `enabled` | boolean | | +| `type` | string | Type of the monitor, such as metric_issue or error | +| `workflowIds` | array \| null | IDs of the alerts connected to the monitor | +| `dateCreated` | string | When the monitor was created (ISO 8601) | +| `dateUpdated` | string | When the monitor was last updated (ISO 8601) | +| `dataSources` | array \| null | Data sources the monitor evaluates, such as a query subscription | +| `conditionGroup` | object \| null | Conditions that decide the monitor's status | +| `config` | object | Configuration of the monitor, which depends on its type | +| `enabled` | boolean | Whether the monitor is enabled | | `org` | string | Organization slug | | `status` | string | Rule status (active, disabled) | diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/event.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/event.md index 0fd78ece8..b4134347d 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/event.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/event.md @@ -30,44 +30,44 @@ View details of one or more events | Field | Type | Description | |-------|------|-------------| -| `id` | string | | -| `groupID` | string \| null | | +| `id` | string | ID of the event | +| `groupID` | string \| null | ID of the issue the event belongs to | | `eventID` | string | UUID-format event ID | -| `projectID` | string | | -| `message` | string \| null | | -| `title` | string | | -| `location` | string \| null | | -| `user` | object \| null | | -| `tags` | array | | -| `platform` | string \| null | | -| `dateReceived` | string \| null | | -| `contexts` | object \| null | | -| `size` | number \| null | | -| `entries` | array | | -| `dist` | string \| null | | -| `sdk` | object \| null | | -| `context` | object \| null | | -| `packages` | object | | -| `type` | string | | -| `metadata` | object | | -| `errors` | array | | +| `projectID` | string | ID of the project the event belongs to | +| `message` | string \| null | Event message | +| `title` | string | Event title | +| `location` | string \| null | Source location of the event, such as a file and line | +| `user` | object \| null | User context of the event | +| `tags` | array | Tags attached to the event | +| `platform` | string \| null | Platform of the event, such as python or javascript | +| `dateReceived` | string \| null | When Sentry received the event (ISO 8601) | +| `contexts` | object \| null | Contexts of the event, such as device, os, browser and runtime | +| `size` | number \| null | Size of the event payload in bytes | +| `entries` | array | Event interfaces, such as exception, breadcrumbs and request, each with a type and data | +| `dist` | string \| null | Distribution of the release the event was sent with | +| `sdk` | object \| null | SDK that sent the event | +| `context` | object \| null | Additional data sent with the event in its extra field | +| `packages` | object | Modules loaded when the event occurred, keyed by module name with their versions | +| `type` | string | Event type, such as error, default or transaction | +| `metadata` | object | Event metadata | +| `errors` | array | Errors Sentry found while processing the event | | `occurrence` | object \| null | | -| `_meta` | object | | -| `crashFile` | string \| null | | -| `culprit` | string \| null | | -| `dateCreated` | string | | -| `fingerprints` | array | | -| `groupingConfig` | object | | -| `startTimestamp` | number | | -| `endTimestamp` | number | | -| `measurements` | object \| null | | -| `breakdowns` | object \| null | | -| `release` | object \| null | | -| `userReport` | object \| null | | -| `sdkUpdates` | array | | -| `resolvedWith` | array | | -| `nextEventID` | string \| null | | -| `previousEventID` | string \| null | | +| `_meta` | object | Metadata about changes Sentry made to the event's fields, such as data scrubbing or trimming | +| `crashFile` | string \| null | Crash report attachment of the event, if any | +| `culprit` | string \| null | Function or module that caused the event | +| `dateCreated` | string | When the event occurred (ISO 8601) | +| `fingerprints` | array | Grouping hashes calculated for the event | +| `groupingConfig` | object | Grouping configuration used to group the event | +| `startTimestamp` | number | When the transaction started, as a Unix timestamp in seconds | +| `endTimestamp` | number | When the transaction ended, as a Unix timestamp in seconds | +| `measurements` | object \| null | Measurements recorded for the transaction, such as web vitals, keyed by name | +| `breakdowns` | object \| null | Span operation breakdowns for the transaction, keyed by breakdown name | +| `release` | object \| null | Release the event was sent with. Null when the event has no release | +| `userReport` | object \| null | User feedback submitted for the event. Null when there's none | +| `sdkUpdates` | array | Suggested updates for the SDK that sent the event | +| `resolvedWith` | array | How the event's stack frames were resolved during processing, such as which source map lookup method was used | +| `nextEventID` | string \| null | ID of the next event in the issue. Null for the latest event or when adjacentEvents is collapsed | +| `previousEventID` | string \| null | ID of the previous event in the issue. Null for the first event or when adjacentEvents is collapsed | | `formatted` | object | Event rendered as Markdown, XML, or JSON; only returned for API requests with llmFormat | | `trace` | object \| null | Trace context, or null when unavailable | | `attachments` | array | Event attachments; each includes metadata and an absolute authenticated download URL | diff --git a/packages/cli/src/lib/objectstore.ts b/packages/cli/src/lib/objectstore.ts index 813d1dc30..25d0708da 100644 --- a/packages/cli/src/lib/objectstore.ts +++ b/packages/cli/src/lib/objectstore.ts @@ -1,415 +1,64 @@ /** - * Minimal Objectstore HTTP client for snapshot image uploads. + * Objectstore uploads for `snapshots upload`. * - * Replicates the subset of the `objectstore-client` protocol (getsentry/objectstore) - * that `snapshots upload` needs: check which objects already exist (dedup) and - * upload the missing ones. The service URL, scopes, auth token, and expiration - * policy all come from the Sentry `snapshots/upload-options/` endpoint — this - * client never signs tokens itself (the token is a pre-signed JWT). - * - * Object path layout: `{serviceUrl}/v1/objects/{usecase}/{scope}/{key}` where - * `scope` is a `;`-joined list of ordered `key=value` pairs. Auth is carried in - * the `x-os-auth: Bearer ` header (not the standard `Authorization`). - * - * Like the Rust client's `Session::many()`, HEADs and small PUTs are grouped - * into multipart requests to `{serviceUrl}/v1/objects:batch/{usecase}/{scope}/`. - * The Sentry proxy in front of Objectstore rate-limits per HTTP request, so one - * request per image quickly trips it (HTTP 429) on larger snapshot suites. + * The protocol client (batched HEADs and inserts, concurrency limits, key + * encoding) lives in `@sentry/api/client`. This module adapts it to the CLI: + * files are read from disk only when their request starts, requests go + * through {@link customFetch} so custom CAs apply, and failures become CLI + * {@link ApiError}s. */ -import { randomUUID } from "node:crypto"; import { readFile, stat } from "node:fs/promises"; -import pLimit from "p-limit"; +import { + findExistingObjects as findExisting, + type ObjectstoreConfig, + ObjectstoreError, + putObjects as putShared, +} from "@sentry/api/client"; import { customFetch } from "./custom-ca.js"; import { ApiError } from "./errors.js"; -/** Header carrying the Objectstore bearer token. */ -const AUTH_HEADER = "x-os-auth"; -/** Header carrying an object's expiration policy (e.g. `ttl:30d`). */ -const EXPIRATION_HEADER = "x-sn-expiration"; - -/** Matches one or more trailing slashes (for base-URL normalization). */ -const TRAILING_SLASHES = /\/+$/; - -/** Timeout for uploading a single object. */ -const PUT_TIMEOUT_MS = 120_000; -/** Timeout for one batch request (up to {@link MAX_BATCH_BODY_SIZE} bytes). */ -const BATCH_TIMEOUT_MS = 300_000; - -/** Per-part headers of the batch protocol. */ -const BATCH_KIND_HEADER = "x-sn-batch-operation-kind"; -const BATCH_KEY_HEADER = "x-sn-batch-operation-key"; -const BATCH_INDEX_HEADER = "x-sn-batch-operation-index"; -const BATCH_STATUS_HEADER = "x-sn-batch-operation-status"; - -/** Server-enforced maximum number of operations per batch request. */ -const MAX_BATCH_OPS = 1000; -/** Server-enforced maximum body size of one batch part; larger objects are PUT individually. */ -const MAX_BATCH_PART_SIZE = 1024 * 1024; -/** - * Maximum total body size of one batch request. With {@link BATCH_CONCURRENCY} - * batches in flight, upload bodies held in memory stay around 100 MB. - */ -const MAX_BATCH_BODY_SIZE = 25 * 1024 * 1024; -/** - * Batch requests in flight at once. Each batch is one request to the Sentry - * proxy, so this stays far below its per-request rate limit. - */ -const BATCH_CONCURRENCY = 4; -/** Concurrency for objects too large to batch (matches the Rust client). */ -const INDIVIDUAL_CONCURRENCY = 5; - -/** Matches the `boundary` parameter of a multipart `Content-Type`. */ -const BOUNDARY_PARAM = /boundary=(?:"([^"]+)"|([^;\s]+))/i; -/** Matches the CRLF that terminates a multipart part body. */ -const TRAILING_CRLF = /\r\n$/; - -/** - * Objectstore upload configuration, as returned (camelCase) by the Sentry - * `snapshots/upload-options/` endpoint. - */ -export type ObjectstoreConfig = { - /** Base service URL (may include a path prefix). */ - url: string; - usecase: string; - /** Ordered scope pairs (e.g. `[["org","1"],["project","2"]]`). */ - scopes: [string, string][]; - /** Pre-signed bearer token, or null/absent for unauthenticated stores. */ - authToken?: string | null; - /** Expiration policy string applied to uploaded objects. */ - expirationPolicy: string; -}; - -/** Render scope pairs into the `k=v;k=v` path segment. */ -function scopeSegment(scopes: [string, string][]): string { - return scopes.map(([key, value]) => `${key}=${value}`).join(";"); -} - -/** - * Build the full URL for an object key within the configured usecase + scope. - * - * @param config - The objectstore configuration. - * @param key - The object key (e.g. `//`). - */ -export function buildObjectUrl(config: ObjectstoreConfig, key: string): string { - const base = config.url.replace(TRAILING_SLASHES, ""); - return `${base}/v1/objects/${config.usecase}/${scopeSegment( - config.scopes, - )}/${key}`; -} - -/** Build the batch endpoint URL for the configured usecase + scope. */ -function buildBatchUrl(config: ObjectstoreConfig): string { - const base = config.url.replace(TRAILING_SLASHES, ""); - return `${base}/v1/objects:batch/${config.usecase}/${scopeSegment( - config.scopes, - )}/`; -} - -/** - * Escape a value for an Objectstore header, matching `encode_header_value` in - * `objectstore-types`: percent-encode control bytes, DEL, `%`, and non-ASCII. - */ -export function encodeHeaderValue(value: string): string { - let out = ""; - for (const byte of Buffer.from(value, "utf8")) { - if (byte < 0x20 || byte >= 0x7f || byte === 0x25) { - out += `%${byte.toString(16).toUpperCase().padStart(2, "0")}`; - } else { - out += String.fromCharCode(byte); - } - } - return out; -} - -/** Auth headers for an objectstore request, if a token is configured. */ -function authHeaders(config: ObjectstoreConfig): Record { - return config.authToken - ? { [AUTH_HEADER]: `Bearer ${config.authToken}` } - : {}; -} - -/** - * Upload an object's bytes (raw — no content encoding). - * - * The key is derived from the original file's SHA-256, so dedup via - * {@link findExistingObjects} is independent of any upload compression; images are - * already-compressed formats, so storing them raw avoids pointless CPU. - * - * @throws {ApiError} On a non-2xx response. - */ -export async function putObject( - config: ObjectstoreConfig, - key: string, - body: Uint8Array, -): Promise { - const url = buildObjectUrl(config, key); - const response = await customFetch(url, { - method: "PUT", - headers: { - ...authHeaders(config), - [EXPIRATION_HEADER]: config.expirationPolicy, - }, - body, - signal: AbortSignal.timeout(PUT_TIMEOUT_MS), - }); - if (!response.ok) { - throw new ApiError( - "Objectstore upload failed", - response.status, - response.statusText || "PUT failed", - url, - ); - } -} - -/** One operation within a batch request. */ -type BatchOperation = - | { kind: "head"; key: string } - | { kind: "insert"; key: string; body: Uint8Array }; - -/** Outcome of one batch operation, as reported in its response part. */ -type BatchOperationResult = { - /** HTTP status of this operation (e.g. 200, 201, 404, 429). */ - status: number; - /** Response part body (an error description for failed operations). */ - message: string; -}; - -/** Serialize batch operations into a `multipart/form-data` request body. */ -function encodeBatchBody( - config: ObjectstoreConfig, - operations: BatchOperation[], - boundary: string, -): Buffer { - const chunks: Uint8Array[] = []; - for (const op of operations) { - let head = - `--${boundary}\r\n` + - 'content-disposition: form-data; name="part"\r\n' + - `${BATCH_KIND_HEADER}: ${op.kind}\r\n` + - `${BATCH_KEY_HEADER}: ${encodeHeaderValue(op.key)}\r\n`; - if (op.kind === "insert") { - head += `${EXPIRATION_HEADER}: ${config.expirationPolicy}\r\n`; - } - chunks.push(Buffer.from(`${head}\r\n`)); - if (op.kind === "insert") { - chunks.push(op.body); - } - chunks.push(Buffer.from("\r\n")); - } - chunks.push(Buffer.from(`--${boundary}--\r\n`)); - return Buffer.concat(chunks); -} - -/** Parse the header block of one multipart part into a lowercase-keyed map. */ -function parsePartHeaders(block: string): Map { - const headers = new Map(); - for (const line of block.split("\r\n")) { - const colon = line.indexOf(":"); - if (colon > 0) { - headers.set( - line.slice(0, colon).trim().toLowerCase(), - line.slice(colon + 1).trim(), - ); - } - } - return headers; -} - -/** - * Parse a batch `multipart/form-data` response into per-operation results, - * keyed by the operation index the server echoes on each part. - * - * Response parts only carry status headers and small error bodies, so the - * whole response is buffered and split on the boundary. - */ -function parseBatchResponse( - contentType: string, - body: Buffer, - url: string, -): Map { - const match = BOUNDARY_PARAM.exec(contentType); - const boundary = match?.[1] ?? match?.[2]; - if (!boundary) { - throw new ApiError( - "Objectstore batch response is not multipart", - 502, - `Unexpected Content-Type: ${contentType || "(none)"}`, - url, - ); - } - - const results = new Map(); - // latin1 maps bytes 1:1, so offsets stay valid for binary-safe splitting. - const segments = body.toString("latin1").split(`--${boundary}`); - for (const segment of segments.slice(1)) { - if (segment.startsWith("--")) { - break; // closing delimiter - } - const part = segment.startsWith("\r\n") ? segment.slice(2) : segment; - const split = part.indexOf("\r\n\r\n"); - if (split < 0) { - continue; - } - const headers = parsePartHeaders(part.slice(0, split)); - const index = Number.parseInt(headers.get(BATCH_INDEX_HEADER) ?? "", 10); - const status = Number.parseInt(headers.get(BATCH_STATUS_HEADER) ?? "", 10); - if (Number.isNaN(index) || Number.isNaN(status)) { - continue; - } - const rawBody = part.slice(split + 4).replace(TRAILING_CRLF, ""); - results.set(index, { - status, - message: Buffer.from(rawBody, "latin1").toString("utf8"), - }); - } - return results; -} +export type { ObjectstoreConfig }; -/** - * Send operations as one batch request. - * - * @returns Results aligned with `operations` by index. - * @throws {ApiError} When the batch request itself fails or the response is - * missing a result for any operation. - */ -async function sendBatch( - config: ObjectstoreConfig, - operations: BatchOperation[], -): Promise { - const url = buildBatchUrl(config); - const boundary = `sentry-cli-${randomUUID()}`; - const response = await customFetch(url, { - method: "POST", - headers: { - ...authHeaders(config), - "content-type": `multipart/form-data; boundary="${boundary}"`, - }, - body: encodeBatchBody(config, operations, boundary), - signal: AbortSignal.timeout(BATCH_TIMEOUT_MS), - }); - if (!response.ok) { - throw new ApiError( - "Objectstore batch request failed", - response.status, - response.statusText || "batch request failed", - url, - ); - } +/** A file to upload under an object key. */ +export type ObjectUpload = { key: string; path: string }; - const parsed = parseBatchResponse( - response.headers.get("content-type") ?? "", - Buffer.from(await response.arrayBuffer()), - url, - ); - return operations.map((op, index) => { - const result = parsed.get(index); - if (!result) { - throw new ApiError( - "Objectstore batch response is incomplete", - 502, - `No result for ${op.kind} of ${op.key}`, - url, +const options = { fetch: customFetch }; + +/** Rethrow an {@link ObjectstoreError} as a CLI {@link ApiError}. */ +async function withCliErrors(work: Promise): Promise { + try { + return await work; + } catch (error) { + if (error instanceof ObjectstoreError) { + const cliError = new ApiError( + error.message, + error.status ?? 0, + error.detail, + error.url, ); + cliError.cause = error; + throw cliError; } - return result; - }); -} - -/** Throw an {@link ApiError} for a failed operation inside a batch. */ -function throwOperationError( - config: ObjectstoreConfig, - message: string, - key: string, - result: BatchOperationResult, -): never { - throw new ApiError( - message, - result.status, - result.message || `HTTP ${result.status}`, - buildObjectUrl(config, key), - ); -} - -/** Split `items` into consecutive chunks of at most `size` items. */ -function chunk(items: T[], size: number): T[][] { - const chunks: T[][] = []; - for (let i = 0; i < items.length; i += size) { - chunks.push(items.slice(i, i + size)); + throw error; } - return chunks; } /** - * Find which keys already exist, using batched HEAD operations (one request - * per {@link MAX_BATCH_OPS} keys, up to {@link BATCH_CONCURRENCY} at once). + * Find which keys already exist. * * @returns The subset of `keys` that exist. * @throws {ApiError} On a failed batch, or any HEAD result other than 2xx/404. */ -export async function findExistingObjects( +export function findExistingObjects( config: ObjectstoreConfig, keys: string[], ): Promise> { - const existing = new Set(); - const limit = pLimit(BATCH_CONCURRENCY); - await Promise.all( - chunk(keys, MAX_BATCH_OPS).map((batch) => - limit(async () => { - const results = await sendBatch( - config, - batch.map((key) => ({ kind: "head", key })), - ); - results.forEach((result, i) => { - const key = batch[i] as string; - if (result.status === 404) { - return; - } - if (result.status >= 400) { - throwOperationError(config, "Objectstore HEAD failed", key, result); - } - existing.add(key); - }); - }), - ), - ); - return existing; -} - -/** A file to upload under an object key. */ -export type ObjectUpload = { key: string; path: string }; - -/** Group sized uploads into batches within the op-count and body-size limits. */ -function groupBatches( - uploads: (ObjectUpload & { size: number })[], -): ObjectUpload[][] { - const batches: ObjectUpload[][] = []; - let current: ObjectUpload[] = []; - let currentSize = 0; - for (const upload of uploads) { - if ( - current.length >= MAX_BATCH_OPS || - (current.length > 0 && currentSize + upload.size > MAX_BATCH_BODY_SIZE) - ) { - batches.push(current); - current = []; - currentSize = 0; - } - current.push(upload); - currentSize += upload.size; - } - if (current.length > 0) { - batches.push(current); - } - return batches; + return withCliErrors(findExisting(config, keys, options)); } /** - * Upload files as objects. Files up to {@link MAX_BATCH_PART_SIZE} are sent in - * batch requests, up to {@link BATCH_CONCURRENCY} at once; larger files are PUT - * individually, up to {@link INDIVIDUAL_CONCURRENCY} at once. Both run in - * parallel. A file is read only when its request starts. + * Upload files as objects. Each file is read only when its request starts. * * @throws {ApiError} On a failed batch, or any failed insert. */ @@ -418,44 +67,11 @@ export async function putObjects( uploads: ObjectUpload[], ): Promise { const sized = await Promise.all( - uploads.map(async (upload) => ({ - ...upload, - size: (await stat(upload.path)).size, + uploads.map(async ({ key, path }) => ({ + key, + size: (await stat(path)).size, + read: () => readFile(path), })), ); - const batchable = sized.filter((u) => u.size <= MAX_BATCH_PART_SIZE); - const individual = sized.filter((u) => u.size > MAX_BATCH_PART_SIZE); - - const batchLimit = pLimit(BATCH_CONCURRENCY); - const individualLimit = pLimit(INDIVIDUAL_CONCURRENCY); - await Promise.all([ - ...groupBatches(batchable).map((batch) => - batchLimit(async () => { - const operations = await Promise.all( - batch.map(async (upload): Promise => ({ - kind: "insert", - key: upload.key, - body: await readFile(upload.path), - })), - ); - const results = await sendBatch(config, operations); - results.forEach((result, i) => { - if (result.status >= 400) { - const key = (batch[i] as ObjectUpload).key; - throwOperationError( - config, - "Objectstore upload failed", - key, - result, - ); - } - }); - }), - ), - ...individual.map((upload) => - individualLimit(async () => { - await putObject(config, upload.key, await readFile(upload.path)); - }), - ), - ]); + await withCliErrors(putShared(config, sized, options)); } diff --git a/packages/cli/test/lib/objectstore.test.ts b/packages/cli/test/lib/objectstore.test.ts index 6587ea9a9..f6c5037f4 100644 --- a/packages/cli/test/lib/objectstore.test.ts +++ b/packages/cli/test/lib/objectstore.test.ts @@ -1,9 +1,9 @@ /** - * Tests for the minimal Objectstore HTTP client. + * Tests for the CLI adapter over the `@sentry/api/client` Objectstore client. * - * `customFetch` is mocked so URL construction, the `x-os-auth` header, the - * batch request/response protocol, and PUT request shape can be verified - * without a network. + * The batch protocol itself is tested in `@sentry/api`. These tests check what + * the CLI adds: requests go through `customFetch`, files are read from disk, + * and failures surface as CLI `ApiError`s. */ import { mkdtemp, rm, writeFile } from "node:fs/promises"; @@ -16,11 +16,8 @@ const { customFetchMock } = vi.hoisted(() => ({ customFetchMock: vi.fn() })); vi.mock("../../src/lib/custom-ca.js", () => ({ customFetch: customFetchMock })); import { - buildObjectUrl, - encodeHeaderValue, findExistingObjects, type ObjectstoreConfig, - putObject, putObjects, } from "../../src/lib/objectstore.js"; @@ -35,6 +32,9 @@ const config: ObjectstoreConfig = { expirationPolicy: "ttl:30d", }; +const BATCH_URL = + "https://objectstore.example.com/v1/objects:batch/preprod_snapshots/org=123;project=456/"; + beforeEach(() => { customFetchMock.mockReset(); }); @@ -42,16 +42,6 @@ afterEach(() => { vi.restoreAllMocks(); }); -describe("buildObjectUrl", () => { - test("joins usecase, scope, and key (stripping a trailing slash)", () => { - expect( - buildObjectUrl({ ...config, usecase: "preprod" }, "123/456/abc"), - ).toBe( - "https://objectstore.example.com/v1/objects/preprod/org=123;project=456/123/456/abc", - ); - }); -}); - /** Build a batch response in the server's `multipart/form-data` shape. */ function batchResponse( parts: { index: number; status: string; body?: string }[], @@ -75,24 +65,15 @@ function batchResponse( /** Decode a captured batch request body as multipart form data. */ async function sentParts(init: RequestInit): Promise { - const headers = init.headers as Record; + const headers = new Headers(init.headers); const form = await new Response(init.body as BodyInit, { - headers: { "content-type": headers["content-type"] as string }, + headers: { "content-type": headers.get("content-type") as string }, }).formData(); return form.getAll("part"); } -describe("encodeHeaderValue", () => { - test("leaves visible ASCII alone and escapes %, controls, and non-ASCII", () => { - expect(encodeHeaderValue("1/2/abc")).toBe("1/2/abc"); - expect(encodeHeaderValue("100% done")).toBe("100%25 done"); - expect(encodeHeaderValue("r\u00e9port")).toBe("r%C3%A9port"); - expect(encodeHeaderValue("a\nb")).toBe("a%0Ab"); - }); -}); - describe("findExistingObjects", () => { - test("sends all HEADs in one batch request and returns existing keys", async () => { + test("sends HEADs as one batch through customFetch", async () => { customFetchMock.mockResolvedValue( batchResponse([ { index: 1, status: "404 Not Found" }, @@ -106,70 +87,39 @@ describe("findExistingObjects", () => { expect([...existing].sort()).toEqual(["k0", "k2"]); expect(customFetchMock).toHaveBeenCalledTimes(1); const [url, init] = customFetchMock.mock.calls[0] ?? []; - expect(url).toBe( - "https://objectstore.example.com/v1/objects:batch/preprod_snapshots/org=123;project=456/", - ); + expect(url).toBe(BATCH_URL); expect(init.method).toBe("POST"); - expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); + expect(new Headers(init.headers).get("x-os-auth")).toBe("Bearer jwt-token"); expect(await sentParts(init)).toHaveLength(3); - const raw = Buffer.from(init.body).toString(); - expect(raw).toContain("x-sn-batch-operation-kind: head\r\n"); - expect(raw).toContain("x-sn-batch-operation-key: k1\r\n"); }); - test("splits more than 1000 keys into batches sent concurrently", async () => { - let inFlight = 0; - let maxInFlight = 0; - customFetchMock.mockImplementation(async (_url, init: RequestInit) => { - inFlight += 1; - maxInFlight = Math.max(maxInFlight, inFlight); - const count = (await sentParts(init)).length; - await new Promise((resolve) => setTimeout(resolve, 10)); - inFlight -= 1; - return batchResponse( - Array.from({ length: count }, (_, index) => ({ - index, - status: "200 OK", - })), - ); - }); - const keys = Array.from({ length: 2500 }, (_, i) => `k${i}`); + test("throws a CLI ApiError with the request URL when the batch fails", async () => { + customFetchMock.mockResolvedValue( + new Response(null, { status: 429, statusText: "Too Many Requests" }), + ); - const existing = await findExistingObjects(config, keys); + const error = await findExistingObjects(config, ["k0"]).catch((e) => e); - expect(existing.size).toBe(2500); - expect(customFetchMock).toHaveBeenCalledTimes(3); - expect(maxInFlight).toBe(3); + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(429); + expect(error.endpoint).toBe(BATCH_URL); }); - test("throws on a per-operation error other than 404", async () => { + test("throws a CLI ApiError for a failed operation inside a batch", async () => { customFetchMock.mockResolvedValue( batchResponse([ { index: 0, status: "200 OK" }, { index: 1, status: "429 Too Many Requests", body: "slow down" }, ]), ); - await expect(findExistingObjects(config, ["k0", "k1"])).rejects.toThrow( - ApiError, - ); - }); - test("throws when the batch request itself fails", async () => { - customFetchMock.mockResolvedValue( - new Response(null, { status: 429, statusText: "Too Many Requests" }), + const error = await findExistingObjects(config, ["k0", "k1"]).catch( + (e) => e, ); - const error = await findExistingObjects(config, ["k0"]).catch((e) => e); + expect(error).toBeInstanceOf(ApiError); expect(error.status).toBe(429); - }); - - test("throws when the response is missing an operation", async () => { - customFetchMock.mockResolvedValue( - batchResponse([{ index: 0, status: "200 OK" }]), - ); - await expect(findExistingObjects(config, ["k0", "k1"])).rejects.toThrow( - ApiError, - ); + expect(error.endpoint).toMatch(/\/v1\/objects\/preprod_snapshots\/.*\/k1$/); }); }); @@ -191,7 +141,7 @@ describe("putObjects", () => { customFetchMock.mockImplementation(async (_url, init: RequestInit) => init.method === "POST" ? batchResponse([{ index: 0, status: "201 Created" }]) - : { ok: true, status: 201 }, + : new Response(null, { status: 201 }), ); await putObjects(config, [ @@ -200,17 +150,13 @@ describe("putObjects", () => { ]); expect(customFetchMock).toHaveBeenCalledTimes(2); - const [, batchInit] = customFetchMock.mock.calls.find( + const [batchUrl, batchInit] = customFetchMock.mock.calls.find( ([, init]) => init.method === "POST", ) ?? [undefined, undefined]; + expect(batchUrl).toBe(BATCH_URL); const parts = await sentParts(batchInit); - expect(parts).toHaveLength(1); // Parts without a filename decode as strings. - expect(parts[0]).toBe("\x01\x02\x03"); - const raw = Buffer.from(batchInit.body).toString("latin1"); - expect(raw).toContain("x-sn-batch-operation-kind: insert\r\n"); - expect(raw).toContain("x-sn-batch-operation-key: 1/2/small\r\n"); - expect(raw).toContain("x-sn-expiration: ttl:30d\r\n"); + expect(parts).toEqual(["\x01\x02\x03"]); const [putUrl, putInit] = customFetchMock.mock.calls.find( ([, init]) => init.method === "PUT", @@ -221,43 +167,25 @@ describe("putObjects", () => { expect(putInit.body.byteLength).toBe(1024 * 1024 + 1); }); - test("throws when an insert in the batch fails", async () => { + test("throws a CLI ApiError when an insert in the batch fails", async () => { const small = join(dir, "small.png"); await writeFile(small, new Uint8Array([1])); customFetchMock.mockResolvedValue( batchResponse([{ index: 0, status: "500 Internal Server Error" }]), ); - await expect( - putObjects(config, [{ key: "k", path: small }]), - ).rejects.toThrow(ApiError); - }); -}); -describe("putObject", () => { - test("PUTs the body with auth + expiration headers", async () => { - customFetchMock.mockResolvedValue({ ok: true, status: 200 }); - const body = new Uint8Array([1, 2, 3]); - await putObject(config, "123/456/abc", body); - - const [url, init] = customFetchMock.mock.calls[0] ?? []; - expect(url).toBe( - "https://objectstore.example.com/v1/objects/preprod_snapshots/org=123;project=456/123/456/abc", + const error = await putObjects(config, [{ key: "k", path: small }]).catch( + (e) => e, ); - expect(init.method).toBe("PUT"); - expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); - expect(init.headers["x-sn-expiration"]).toBe("ttl:30d"); - expect(init.body).toBe(body); - expect(init.signal).toBeInstanceOf(AbortSignal); + + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(500); }); - test("throws on a non-2xx response", async () => { - customFetchMock.mockResolvedValue({ - ok: false, - status: 413, - statusText: "too large", - }); - await expect(putObject(config, "k", new Uint8Array([0]))).rejects.toThrow( - ApiError, - ); + test("fails before any request when a file is missing", async () => { + await expect( + putObjects(config, [{ key: "k", path: join(dir, "missing.png") }]), + ).rejects.toThrow(/ENOENT/); + expect(customFetchMock).not.toHaveBeenCalled(); }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b0527ff5c..fb33914d8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -43,8 +43,8 @@ catalogs: specifier: ^1.2.3 version: 1.2.3 '@sentry/api': - specifier: 1.26.0 - version: 1.26.0 + specifier: 1.44.0 + version: 1.44.0 '@sentry/cloudflare': specifier: 11.6.0 version: 11.6.0 @@ -513,7 +513,7 @@ importers: version: 2.1.5(hono@4.13.13) '@sentry/api': specifier: 'catalog:' - version: 1.26.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) + version: 1.44.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) '@sentry/core': specifier: 'catalog:' version: 11.6.0 @@ -865,7 +865,7 @@ importers: version: 2.4.0 '@sentry/api': specifier: 'catalog:' - version: 1.26.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) + version: 1.44.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) '@sentry/core': specifier: 'catalog:' version: 11.6.0 @@ -1048,7 +1048,7 @@ importers: dependencies: '@sentry/api': specifier: 'catalog:' - version: 1.26.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) + version: 1.44.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) '@sentry/search-syntax': specifier: 'catalog:' version: 1.0.0 @@ -4275,8 +4275,8 @@ packages: '@sec-ant/readable-stream@0.4.1': resolution: {integrity: sha512-831qok9r2t8AlxLko40y2ebgSDhenenCatLVeW/uBtnHPyhHOvG0C7TvfgecV+wHzIm5KUICgzmVpWS+IMEAeg==} - '@sentry/api@1.26.0': - resolution: {integrity: sha512-LkzMT2mn2Qrbnf2s+2t/mrXpN1/2o0GO2fodhds2Ntp8kRt/NkilWIETAGUU6vB7XKlLhPZloZT4Yv/HEzCmFA==} + '@sentry/api@1.44.0': + resolution: {integrity: sha512-8PPlt5YuYLOn3btCatpogIa+Q2NW9dmtB5JPJA6IF7fKNLPUHqkojVBfOSNUb9fD5UE0ZxUzZsBUnVhW5/JvrQ==} engines: {node: '>=22'} peerDependencies: typescript: '*' @@ -7691,6 +7691,10 @@ packages: resolution: {integrity: sha512-Ll0w3fU24vYpXoZmjjZIee6bJQDgG0oAyo1PdmFYI8UDwJJddaHAypxIH9avUu+t+lSsAwKVsb1jDCMIIChliw==} engines: {node: '>=20'} + p-limit@7.3.3: + resolution: {integrity: sha512-SKgVvDU5TNIxQ4r30U75BNLC6a+6GOEjngrk/ysM5+Zu3oJuk1SC5ApzIzY/7PjCEjoVPMWJ0zho/4QUiEB7TQ==} + engines: {node: '>=20'} + p-locate@5.0.0: resolution: {integrity: sha512-LaNjtRWUBY++zB5nE/NwcaoMylSPk+S+ZHNB1TzdbMJMny6dynpAGt7X/tl/QYq3TIeE6nxHppbo2LGymrG5Pw==} engines: {node: '>=10'} @@ -12108,9 +12112,10 @@ snapshots: '@sec-ant/readable-stream@0.4.1': {} - '@sentry/api@1.26.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7)': + '@sentry/api@1.44.0(typescript@5.9.3)(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7)': dependencies: http-cache-semantics: 4.2.0 + p-limit: 7.3.3 optionalDependencies: typescript: 5.9.3 valibot: 1.4.2(typescript@5.9.3) @@ -16149,6 +16154,10 @@ snapshots: dependencies: yocto-queue: 1.2.2 + p-limit@7.3.3: + dependencies: + yocto-queue: 1.2.2 + p-locate@5.0.0: dependencies: p-limit: 3.1.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index aebcf7b2e..d198de786 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -51,7 +51,7 @@ catalog: "@modelcontextprotocol/server": 2.4.0 "@radix-ui/react-accordion": ^1.2.20 "@radix-ui/react-slot": ^1.2.3 - "@sentry/api": 1.26.0 + "@sentry/api": 1.44.0 "@sentry/cloudflare": 11.6.0 "@sentry/core": 11.6.0 "@sentry/node": 11.6.0 @@ -168,7 +168,7 @@ minimumReleaseAgeExclude: - "@oxlint/binding-win32-x64-msvc@1.87.0" - oxfmt@0.72.0 - oxlint@1.87.0 - - "@sentry/api@1.26.0" + - "@sentry/api@1.44.0" - miniflare@5.20261101.0-beta - wrangler@4.156.1 - "@sentry/symbolic@14.0.0-alpha.5"