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 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/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..25d0708da 100644 --- a/packages/cli/src/lib/objectstore.ts +++ b/packages/cli/src/lib/objectstore.ts @@ -1,134 +1,77 @@ /** - * 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: 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). - * - * 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`). + * 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 { readFile, stat } from "node:fs/promises"; +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 a HEAD existence check. */ -const HEAD_TIMEOUT_MS = 30_000; -/** Timeout for uploading a single object. */ -const PUT_TIMEOUT_MS = 120_000; - -/** - * 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; -}; +export type { ObjectstoreConfig }; -/** 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(";"); -} +/** A file to upload under an object key. */ +export type ObjectUpload = { key: string; path: string }; -/** - * 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}`; -} +const options = { fetch: customFetch }; -/** Auth headers for an objectstore request, if a token is configured. */ -function authHeaders(config: ObjectstoreConfig): Record { - return config.authToken - ? { [AUTH_HEADER]: `Bearer ${config.authToken}` } - : {}; +/** 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; + } + throw error; + } } /** - * Check whether an object already exists (used to skip re-uploads). + * Find which keys already exist. * - * @returns `true` if the object exists, `false` on a 404. - * @throws {ApiError} On any non-2xx, non-404 response. + * @returns The subset of `keys` that exist. + * @throws {ApiError} On a failed batch, or any HEAD result other than 2xx/404. */ -export async function objectExists( +export function findExistingObjects( config: ObjectstoreConfig, - key: string, -): Promise { - const url = buildObjectUrl(config, key); - const response = await customFetch(url, { - method: "HEAD", - headers: authHeaders(config), - signal: AbortSignal.timeout(HEAD_TIMEOUT_MS), - }); - if (response.status === 404) { - return false; - } - if (!response.ok) { - throw new ApiError( - "Objectstore HEAD failed", - response.status, - response.statusText || "HEAD failed", - url, - ); - } - return true; + keys: string[], +): Promise> { + return withCliErrors(findExisting(config, keys, options)); } /** - * Upload an object's bytes (raw — no content encoding). + * Upload files as objects. Each file is read only when its request starts. * - * 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. - * - * @throws {ApiError} On a non-2xx response. + * @throws {ApiError} On a failed batch, or any failed insert. */ -export async function putObject( +export async function putObjects( config: ObjectstoreConfig, - key: string, - body: Uint8Array, + uploads: ObjectUpload[], ): 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, - ); - } + const sized = await Promise.all( + uploads.map(async ({ key, path }) => ({ + key, + size: (await stat(path)).size, + read: () => readFile(path), + })), + ); + await withCliErrors(putShared(config, sized, options)); } 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..f6c5037f4 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. + * Tests for the CLI adapter over the `@sentry/api/client` Objectstore 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. + * 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"; +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"; @@ -12,10 +16,9 @@ const { customFetchMock } = vi.hoisted(() => ({ customFetchMock: vi.fn() })); vi.mock("../../src/lib/custom-ca.js", () => ({ customFetch: customFetchMock })); import { - buildObjectUrl, + findExistingObjects, type ObjectstoreConfig, - objectExists, - putObject, + putObjects, } from "../../src/lib/objectstore.js"; const config: ObjectstoreConfig = { @@ -29,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(); }); @@ -36,76 +42,150 @@ 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 }[], +): 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}"` }, }); -}); +} -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); - const [url, init] = customFetchMock.mock.calls[0] ?? []; - expect(url).toContain( - "/v1/objects/preprod_snapshots/org=123;project=456/123/456/abc", +/** Decode a captured batch request body as multipart form data. */ +async function sentParts(init: RequestInit): Promise { + const headers = new Headers(init.headers); + const form = await new Response(init.body as BodyInit, { + headers: { "content-type": headers.get("content-type") as string }, + }).formData(); + return form.getAll("part"); +} + +describe("findExistingObjects", () => { + test("sends HEADs as one batch through customFetch", async () => { + customFetchMock.mockResolvedValue( + batchResponse([ + { index: 1, status: "404 Not Found" }, + { index: 0, status: "200 OK" }, + { index: 2, status: "200 OK" }, + ]), ); - expect(init.method).toBe("HEAD"); - expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); - expect(init.signal).toBeInstanceOf(AbortSignal); - }); - test("returns false on a 404", async () => { - customFetchMock.mockResolvedValue({ ok: false, status: 404 }); - expect(await objectExists(config, "123/456/abc")).toBe(false); + 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).toBe(BATCH_URL); + expect(init.method).toBe("POST"); + expect(new Headers(init.headers).get("x-os-auth")).toBe("Bearer jwt-token"); + expect(await sentParts(init)).toHaveLength(3); }); - 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 a CLI ApiError with the request URL when the batch 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); + expect(error.endpoint).toBe(BATCH_URL); }); - 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 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" }, + ]), + ); + + const error = await findExistingObjects(config, ["k0", "k1"]).catch( + (e) => e, + ); + + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(429); + expect(error.endpoint).toMatch(/\/v1\/objects\/preprod_snapshots\/.*\/k1$/); }); }); -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); +describe("putObjects", () => { + let dir: string; - 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", + 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" }]) + : new Response(null, { status: 201 }), + ); + + await putObjects(config, [ + { key: "1/2/small", path: small }, + { key: "1/2/large", path: large }, + ]); + + expect(customFetchMock).toHaveBeenCalledTimes(2); + const [batchUrl, batchInit] = customFetchMock.mock.calls.find( + ([, init]) => init.method === "POST", + ) ?? [undefined, undefined]; + expect(batchUrl).toBe(BATCH_URL); + const parts = await sentParts(batchInit); + // Parts without a filename decode as strings. + expect(parts).toEqual(["\x01\x02\x03"]); + + const [putUrl, putInit] = customFetchMock.mock.calls.find( + ([, init]) => init.method === "PUT", + ) ?? [undefined, undefined]; + expect(putUrl).toMatch( + /\/v1\/objects\/preprod_snapshots\/.*\/1\/2\/large$/, ); - 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(putInit.body.byteLength).toBe(1024 * 1024 + 1); }); - 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("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" }]), + ); + + const error = await putObjects(config, [{ key: "k", path: small }]).catch( + (e) => e, ); + + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(500); + }); + + 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"