From 13053adab8dc2c7f923926b48d5f5c3bf0825abe Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 16:44:18 +0000 Subject: [PATCH 01/10] feat(toolkit-core): Guard shared modules against Node-only APIs Lint bans node:* imports and Node globals in toolkit-core, and a second typecheck runs it against the DOM library with no Node types. The cache codec keeps node:zlib; its types move to body.ts so the policy and stores stay portable. --- .oxlintrc.json | 171 ++++++++++++++++++ packages/toolkit-core/package.json | 2 +- .../toolkit-core/src/response-cache/body.ts | 34 ++++ .../toolkit-core/src/response-cache/codec.ts | 30 +-- .../toolkit-core/src/response-cache/policy.ts | 2 +- .../toolkit-core/src/response-cache/store.ts | 2 +- packages/toolkit-core/tsconfig.browser.json | 9 + 7 files changed, 219 insertions(+), 31 deletions(-) create mode 100644 packages/toolkit-core/src/response-cache/body.ts create mode 100644 packages/toolkit-core/tsconfig.browser.json diff --git a/.oxlintrc.json b/.oxlintrc.json index 5e913135b..673fcf3ea 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -145,6 +145,177 @@ "rules": { "no-bitwise": "off" } + }, + { + "files": ["packages/toolkit-core/src/**/*.ts"], + "rules": { + "no-restricted-imports": [ + "error", + { + "paths": [ + { + "name": "assert", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "async_hooks", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "buffer", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "child_process", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "crypto", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "dns", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "events", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "fs", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "fs/promises", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "http", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "https", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "module", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "net", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "os", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "path", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "perf_hooks", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "process", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "readline", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "stream", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "stream/promises", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "timers", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "tls", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "url", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "util", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "worker_threads", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "zlib", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + } + ], + "patterns": [ + { + "group": ["node:*"], + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + } + ] + } + ], + "no-restricted-globals": [ + "error", + { + "name": "Buffer", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "process", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "global", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "require", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "module", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "__dirname", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "__filename", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "setImmediate", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "clearImmediate", + "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + } + ] + } + }, + { + "files": [ + "packages/toolkit-core/src/**/*.test.ts", + "packages/toolkit-core/src/response-cache/codec.ts" + ], + "rules": { + "no-restricted-imports": "off", + "no-restricted-globals": "off" + } } ] } diff --git a/packages/toolkit-core/package.json b/packages/toolkit-core/package.json index c1c5ab43b..b42fa0cd4 100644 --- a/packages/toolkit-core/package.json +++ b/packages/toolkit-core/package.json @@ -83,7 +83,7 @@ "scripts": { "test": "vitest run", "test:ci": "vitest run --reporter=default --reporter=junit --outputFile=tests.junit.xml", - "tsc": "tsc --noEmit" + "tsc": "tsc --noEmit && tsc -p tsconfig.browser.json" }, "dependencies": { "@sentry/search-syntax": "catalog:", diff --git a/packages/toolkit-core/src/response-cache/body.ts b/packages/toolkit-core/src/response-cache/body.ts new file mode 100644 index 000000000..6403d650d --- /dev/null +++ b/packages/toolkit-core/src/response-cache/body.ts @@ -0,0 +1,34 @@ +/** + * The shape of a stored response body and of the codec that produces it. + * Kept apart from `codec.ts`, which needs `node:zlib`, so the cache policy + * and stores stay portable. + */ + +/** How an entry's body bytes are encoded. */ +export type BodyEncoding = "zstd" | "br" | "identity"; + +/** A body as stored: its bytes plus what is needed to decode them. */ +export type EncodedBody = { + bytes: Uint8Array; + encoding: BodyEncoding; + /** + * Compression dictionary the bytes need. Never set today; recorded so a + * later dictionary rollout can tell its entries apart from plain ones. + */ + dictId?: string; + /** Size of the original body, which bounds decompression. */ + rawSize: number; +}; + +/** Encodes and decodes cached bodies. */ +export type BodyCodec = { + /** Compress a body for storage. */ + encode(raw: Uint8Array): Promise; + /** + * Restore a stored body. Returns `undefined` when this runtime cannot read + * the entry (zstd on Node < 22.15, or a dictionary it does not have), so the + * caller treats it as a miss. Throws when the bytes are corrupt or do not + * decode to `rawSize` bytes. + */ + decode(body: EncodedBody): Promise; +}; diff --git a/packages/toolkit-core/src/response-cache/codec.ts b/packages/toolkit-core/src/response-cache/codec.ts index 1a9a5a8a4..782910a27 100644 --- a/packages/toolkit-core/src/response-cache/codec.ts +++ b/packages/toolkit-core/src/response-cache/codec.ts @@ -11,22 +11,9 @@ // A namespace import, so a missing `zstdCompress` (Node < 22.15) is // `undefined` rather than a link error. import * as nodeZlib from "node:zlib"; +import type { BodyCodec, EncodedBody } from "./body.js"; -/** How an entry's body bytes are encoded. */ -export type BodyEncoding = "zstd" | "br" | "identity"; - -/** A body as stored: its bytes plus what is needed to decode them. */ -export type EncodedBody = { - bytes: Uint8Array; - encoding: BodyEncoding; - /** - * Compression dictionary the bytes need. Never set today; recorded so a - * later dictionary rollout can tell its entries apart from plain ones. - */ - dictId?: string; - /** Size of the original body, which bounds decompression. */ - rawSize: number; -}; +export type { BodyCodec, BodyEncoding, EncodedBody } from "./body.js"; /** Bodies smaller than this are stored uncompressed. */ export const MIN_COMPRESSED_BYTES = 256; @@ -53,19 +40,6 @@ export type ZlibRuntime = { zstdDecompress?: Transform; }; -/** Encodes and decodes cached bodies. */ -export type BodyCodec = { - /** Compress a body for storage. */ - encode(raw: Uint8Array): Promise; - /** - * Restore a stored body. Returns `undefined` when this runtime cannot read - * the entry (zstd on Node < 22.15, or a dictionary it does not have), so the - * caller treats it as a miss. Throws when the bytes are corrupt or do not - * decode to `rawSize` bytes. - */ - decode(body: EncodedBody): Promise; -}; - function run( transform: Transform, input: Uint8Array, diff --git a/packages/toolkit-core/src/response-cache/policy.ts b/packages/toolkit-core/src/response-cache/policy.ts index ba3a74136..c9e477f25 100644 --- a/packages/toolkit-core/src/response-cache/policy.ts +++ b/packages/toolkit-core/src/response-cache/policy.ts @@ -10,7 +10,7 @@ */ import CachePolicy from "http-cache-semantics"; -import type { BodyCodec } from "./codec.js"; +import type { BodyCodec } from "./body.js"; import type { CacheEntry, CacheStore } from "./store.js"; import { classifyUrl, FALLBACK_TTL_MS } from "./tiers.js"; diff --git a/packages/toolkit-core/src/response-cache/store.ts b/packages/toolkit-core/src/response-cache/store.ts index 72a712ad1..57f704086 100644 --- a/packages/toolkit-core/src/response-cache/store.ts +++ b/packages/toolkit-core/src/response-cache/store.ts @@ -4,7 +4,7 @@ * `response-cache/codec`, and each enforces its own byte budget. */ -import type { EncodedBody } from "./codec.js"; +import type { EncodedBody } from "./body.js"; import type { TtlTier } from "./tiers.js"; /** A cached response as a store holds it. */ diff --git a/packages/toolkit-core/tsconfig.browser.json b/packages/toolkit-core/tsconfig.browser.json new file mode 100644 index 000000000..6c13f0c7a --- /dev/null +++ b/packages/toolkit-core/tsconfig.browser.json @@ -0,0 +1,9 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "lib": ["ES2022", "DOM", "DOM.Iterable"], + "types": [] + }, + "include": ["src/**/*.ts"], + "exclude": ["src/**/*.test.ts", "src/response-cache/codec.ts"] +} From f13df0d24c20643b5d1b89c6e10f5d7969c84ed7 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:21:48 +0000 Subject: [PATCH 02/10] feat(toolkit-core): Validate API responses through Standard Schema validateResponse takes any Standard Schema validator, so the MCP keeps Zod and the CLI keeps valibot. toolkit-core depends only on the interface's types, and lint now bans zod and valibot imports in shared modules. --- .oxlintrc.json | 12 ++++ packages/toolkit-core/package.json | 9 ++- .../src/api-client/validate.test.ts | 55 ++++++++++++++++ .../toolkit-core/src/api-client/validate.ts | 64 +++++++++++++++++++ pnpm-lock.yaml | 9 +++ 5 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 packages/toolkit-core/src/api-client/validate.test.ts create mode 100644 packages/toolkit-core/src/api-client/validate.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index 673fcf3ea..d1b380f4b 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -256,12 +256,24 @@ { "name": "zlib", "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "name": "zod", + "message": "Shared toolkit-core modules validate through the Standard Schema interface; the host passes Zod or valibot schemas in." + }, + { + "name": "valibot", + "message": "Shared toolkit-core modules validate through the Standard Schema interface; the host passes Zod or valibot schemas in." } ], "patterns": [ { "group": ["node:*"], "message": "Shared toolkit-core modules move into @sentry/api and must run in browsers. Use web-standard APIs, or inject the host-specific piece." + }, + { + "group": ["zod/*", "@sentry/api/zod", "@sentry/api/valibot"], + "message": "Shared toolkit-core modules validate through the Standard Schema interface; the host passes Zod or valibot schemas in." } ] } diff --git a/packages/toolkit-core/package.json b/packages/toolkit-core/package.json index b42fa0cd4..c725b2117 100644 --- a/packages/toolkit-core/package.json +++ b/packages/toolkit-core/package.json @@ -78,6 +78,10 @@ "./sentry-origin": { "types": "./src/sentry-origin.ts", "default": "./src/sentry-origin.ts" + }, + "./api-client/*": { + "types": "./src/api-client/*.ts", + "default": "./src/api-client/*.ts" } }, "scripts": { @@ -87,12 +91,15 @@ }, "dependencies": { "@sentry/search-syntax": "catalog:", + "@standard-schema/spec": "1.1.0", "http-cache-semantics": "^4.2.0" }, "devDependencies": { "@types/http-cache-semantics": "^4.2.0", "fast-check": "^4.8.0", "typescript": "catalog:", - "vitest": "catalog:" + "valibot": "^1.4.2", + "vitest": "catalog:", + "zod": "catalog:" } } diff --git a/packages/toolkit-core/src/api-client/validate.test.ts b/packages/toolkit-core/src/api-client/validate.test.ts new file mode 100644 index 000000000..fb6e1044b --- /dev/null +++ b/packages/toolkit-core/src/api-client/validate.test.ts @@ -0,0 +1,55 @@ +import type { StandardSchemaV1 } from "@standard-schema/spec"; +import * as v from "valibot"; +import { describe, expect, it } from "vitest"; +import { z } from "zod"; +import { ApiResponseValidationError, validateResponse } from "./validate.js"; + +describe("validateResponse", () => { + it("returns the output of a Zod schema", () => { + const schema = z.object({ id: z.coerce.number() }); + const result: { id: number } = validateResponse(schema, { id: "7" }); + expect(result).toEqual({ id: 7 }); + }); + + it("returns the output of a valibot schema", () => { + const schema = v.object({ slug: v.string() }); + expect(validateResponse(schema, { slug: "web", extra: 1 })).toEqual({ + slug: "web", + }); + }); + + it("throws with the issues and their paths", () => { + const schema = z.object({ project: z.object({ id: z.string() }) }); + let caught: unknown; + try { + validateResponse(schema, { project: { id: 1 } }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(ApiResponseValidationError); + const error = caught as ApiResponseValidationError; + expect(error.issues).toHaveLength(1); + expect(error.message).toMatch(/^Unexpected API response: project\.id: /); + }); + + it("lists at most three issues in the message", () => { + const schema = v.object({ + a: v.string(), + b: v.string(), + c: v.string(), + d: v.string(), + }); + expect(() => validateResponse(schema, {})).toThrow(/\(and 1 more\)$/); + }); + + it("rejects a schema that validates asynchronously", () => { + const schema: StandardSchemaV1 = { + "~standard": { + version: 1, + vendor: "test", + validate: async (value) => ({ value }), + }, + }; + expect(() => validateResponse(schema, {})).toThrow(TypeError); + }); +}); diff --git a/packages/toolkit-core/src/api-client/validate.ts b/packages/toolkit-core/src/api-client/validate.ts new file mode 100644 index 000000000..43c7d9342 --- /dev/null +++ b/packages/toolkit-core/src/api-client/validate.ts @@ -0,0 +1,64 @@ +/** + * Response validation through the Standard Schema interface, so the shared + * client works with any validator that implements it. The MCP passes Zod + * schemas and the CLI valibot ones; this module depends on neither. + */ + +import type { StandardSchemaV1 } from "@standard-schema/spec"; + +/** A schema the host supplies to check a response body. */ +export type ResponseSchema = StandardSchemaV1< + unknown, + Output +>; + +const MAX_LISTED_ISSUES = 3; + +function issuePath(issue: StandardSchemaV1.Issue): string { + return (issue.path ?? []) + .map((segment) => + String(typeof segment === "object" ? segment.key : segment), + ) + .join("."); +} + +/** A response did not match the schema the caller validated it with. */ +export class ApiResponseValidationError extends Error { + readonly issues: readonly StandardSchemaV1.Issue[]; + + constructor(issues: readonly StandardSchemaV1.Issue[]) { + const listed = issues.slice(0, MAX_LISTED_ISSUES).map((issue) => { + const path = issuePath(issue); + return path ? `${path}: ${issue.message}` : issue.message; + }); + const more = + issues.length > MAX_LISTED_ISSUES + ? ` (and ${issues.length - MAX_LISTED_ISSUES} more)` + : ""; + super(`Unexpected API response: ${listed.join("; ")}${more}`); + this.name = "ApiResponseValidationError"; + this.issues = issues; + } +} + +/** + * Validate `data` with `schema` and return the schema's output. + * + * Responses are validated synchronously, as every caller does today, so a + * schema with async refinements is rejected rather than awaited. + * + * @throws {ApiResponseValidationError} When `data` does not match + */ +export function validateResponse( + schema: Schema, + data: unknown, +): StandardSchemaV1.InferOutput { + const result = schema["~standard"].validate(data); + if (result instanceof Promise) { + throw new TypeError("Response schemas must validate synchronously"); + } + if (result.issues) { + throw new ApiResponseValidationError(result.issues); + } + return result.value; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index eca5ecb84..2633d2bc8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1024,6 +1024,9 @@ importers: '@sentry/search-syntax': specifier: 'catalog:' version: 1.0.0 + '@standard-schema/spec': + specifier: 1.1.0 + version: 1.1.0 http-cache-semantics: specifier: ^4.2.0 version: 4.2.0 @@ -1037,9 +1040,15 @@ importers: typescript: specifier: 'catalog:' version: 5.8.3 + valibot: + specifier: ^1.4.2 + version: 1.4.2(typescript@5.8.3) vitest: specifier: 'catalog:' version: 4.1.11(@opentelemetry/api@1.9.1)(@types/node@24.13.5)(@vitest/coverage-v8@4.1.11)(jsdom@28.1.0)(msw@2.10.2(@types/node@24.13.5)(typescript@5.8.3))(vite@8.3.3(@types/node@24.13.5)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.13)(yaml@2.9.0)) + zod: + specifier: 'catalog:' + version: 4.6.7 packages: From bea21d648c3f13c3d86b88a923dcd5f329841d2f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:45:42 +0000 Subject: [PATCH 03/10] build(mcp-core): Emit ESM only Nothing loads mcp-core's CJS output. The package is private, "type": "module", and all 29 exports point at .js with no "require" condition, so require() would resolve to the ESM files anyway. Its consumers are workspace-only: mcp-server bundles it as a devDependency through those .js exports, mcp-cloudflare bundles it with Vite, and mcp-test-client and mcp-server-evals are private and ESM. No source in the repo requires it or reads its .cjs files. Dropping the CJS build also avoids the rolldown "canonical name not found" panic hit by the CJS pass once the API errors move to toolkit-core. mcp-server's own CJS+ESM output is unchanged. --- packages/mcp-core/scripts/check-build-imports.ts | 9 ++++----- packages/mcp-core/tsdown.config.ts | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/mcp-core/scripts/check-build-imports.ts b/packages/mcp-core/scripts/check-build-imports.ts index 78e4d0305..1a977f51e 100644 --- a/packages/mcp-core/scripts/check-build-imports.ts +++ b/packages/mcp-core/scripts/check-build-imports.ts @@ -1,10 +1,9 @@ import { readFileSync } from "node:fs"; const expectedImport = "@sentry/core/server"; +const path = "dist/server.js"; -for (const path of ["dist/server.js", "dist/server.cjs"]) { - const source = readFileSync(path, "utf8"); - if (!source.includes(expectedImport)) { - throw new Error(`${path} must preserve the ${expectedImport} import`); - } +const source = readFileSync(path, "utf8"); +if (!source.includes(expectedImport)) { + throw new Error(`${path} must preserve the ${expectedImport} import`); } diff --git a/packages/mcp-core/tsdown.config.ts b/packages/mcp-core/tsdown.config.ts index 400f040be..e212ea53f 100644 --- a/packages/mcp-core/tsdown.config.ts +++ b/packages/mcp-core/tsdown.config.ts @@ -7,7 +7,8 @@ const packageVersion = export default defineConfig({ entry: ["src/**/*.ts", "!src/**/*.test.ts"], - format: ["cjs", "esm"], // Build for commonJS and ESmodules + // ESM only: the package is private and every export points at .js. + format: ["esm"], dts: true, // Generate declaration file (.d.ts) sourcemap: true, clean: true, From 0d083bdf3ef4827d569739e6eb87624d20d3998b Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:46:07 +0000 Subject: [PATCH 04/10] ref(toolkit-core): Move API errors and apiPath into the shared client Generated by the ts-morph codemod move-shared-errors.mjs (script in the PR description), run from a scratch directory with ts-morph@28.0.0 installed: node move-shared-errors.mjs oxfmt It moves mcp-core's api-client/errors.ts, api-path.ts, and api-path.test.ts into toolkit-core/src/api-client/, moves UserInputError and ConfigurationError into the shared errors module (mcp-core/src/errors.ts re-exports them), and rewrites every importer. --- .../src/api-client/client-event.test.ts | 2 +- .../mcp-core/src/api-client/client.test.ts | 2 +- packages/mcp-core/src/api-client/client.ts | 4 +-- packages/mcp-core/src/api-client/index.ts | 2 +- packages/mcp-core/src/errors.ts | 26 +++---------------- .../src/tools/support/issue-linking/app.ts | 2 +- .../src/api-client/api-path.test.ts | 4 +-- .../src/api-client/api-path.ts | 4 +-- .../src/api-client/errors.ts | 23 ++++++++++++++++ 9 files changed, 37 insertions(+), 32 deletions(-) rename packages/{mcp-core => toolkit-core}/src/api-client/api-path.test.ts (98%) rename packages/{mcp-core => toolkit-core}/src/api-client/api-path.ts (90%) rename packages/{mcp-core => toolkit-core}/src/api-client/errors.ts (93%) diff --git a/packages/mcp-core/src/api-client/client-event.test.ts b/packages/mcp-core/src/api-client/client-event.test.ts index b70f4c546..9f0d6d6fd 100644 --- a/packages/mcp-core/src/api-client/client-event.test.ts +++ b/packages/mcp-core/src/api-client/client-event.test.ts @@ -3,7 +3,7 @@ import { http, HttpResponse } from "msw"; import { afterEach, describe, expect, it, vi } from "vitest"; import { testEvents } from "../internal/test-fixtures"; import { SentryApiService } from "./client"; -import { ApiValidationError } from "./errors"; +import { ApiValidationError } from "@sentry/toolkit-core/api-client/errors"; const request = { organizationSlug: "test-org", diff --git a/packages/mcp-core/src/api-client/client.test.ts b/packages/mcp-core/src/api-client/client.test.ts index 976f4914f..874b9f40d 100644 --- a/packages/mcp-core/src/api-client/client.test.ts +++ b/packages/mcp-core/src/api-client/client.test.ts @@ -15,7 +15,7 @@ import { ApiRateLimitError, ApiServerError, ApiValidationError, -} from "./errors"; +} from "@sentry/toolkit-core/api-client/errors"; import { z } from "zod"; describe("validateEvents error bodies", () => { diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index b099a9d7b..c69d5661d 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -59,14 +59,14 @@ import { type TraceMetricIdentifier, } from "../utils/url-utils"; import { USER_AGENT } from "../version"; -import { apiPath } from "./api-path"; +import { apiPath } from "@sentry/toolkit-core/api-client/api-path"; import { ApiError, ApiNotFoundError, ApiRateLimitError, ApiValidationError, createApiError, -} from "./errors"; +} from "@sentry/toolkit-core/api-client/errors"; import { AgenticOnboardingRunSchema, AIConversationDetailsResponseSchema, diff --git a/packages/mcp-core/src/api-client/index.ts b/packages/mcp-core/src/api-client/index.ts index e14ac4b10..df47ef807 100644 --- a/packages/mcp-core/src/api-client/index.ts +++ b/packages/mcp-core/src/api-client/index.ts @@ -1,4 +1,4 @@ export * from "./client"; export * from "./schema"; export * from "./types"; -export * from "./errors"; +export * from "@sentry/toolkit-core/api-client/errors"; diff --git a/packages/mcp-core/src/errors.ts b/packages/mcp-core/src/errors.ts index 225d2d824..9323813f4 100644 --- a/packages/mcp-core/src/errors.ts +++ b/packages/mcp-core/src/errors.ts @@ -1,25 +1,7 @@ -/** - * Error thrown when user input validation fails. - * These errors should be returned to the user directly without logging to Sentry. - */ -export class UserInputError extends Error { - constructor(message: string, options?: ErrorOptions) { - super(message, options); - this.name = "UserInputError"; - } -} - -/** - * Error thrown when configuration is invalid or missing. - * These errors should be returned to the user directly without logging to Sentry. - * Typically used for environment configuration issues, connection settings, etc. - */ -export class ConfigurationError extends Error { - constructor(message: string, options?: ErrorOptions) { - super(message, options); - this.name = "ConfigurationError"; - } -} +export { + ConfigurationError, + UserInputError, +} from "@sentry/toolkit-core/api-client/errors"; /** * Error thrown when an LLM provider (OpenAI, Anthropic, OpenRouter, etc.) rejects diff --git a/packages/mcp-core/src/tools/support/issue-linking/app.ts b/packages/mcp-core/src/tools/support/issue-linking/app.ts index d69840c5f..a883e3166 100644 --- a/packages/mcp-core/src/tools/support/issue-linking/app.ts +++ b/packages/mcp-core/src/tools/support/issue-linking/app.ts @@ -1,7 +1,7 @@ /** Resolve installed App forms; every link mutation uses the backend's canonical-URL guard. */ import { z } from "zod"; import type { SentryApiService } from "../../../api-client"; -import { ApiClientError } from "../../../api-client/errors"; +import { ApiClientError } from "@sentry/toolkit-core/api-client/errors"; import { SentryAppExternalRequestOptionsSchema } from "../../../api-client/schema"; import { UserInputError } from "../../../errors"; diff --git a/packages/mcp-core/src/api-client/api-path.test.ts b/packages/toolkit-core/src/api-client/api-path.test.ts similarity index 98% rename from packages/mcp-core/src/api-client/api-path.test.ts rename to packages/toolkit-core/src/api-client/api-path.test.ts index 1266b2787..266c074f7 100644 --- a/packages/mcp-core/src/api-client/api-path.test.ts +++ b/packages/toolkit-core/src/api-client/api-path.test.ts @@ -1,7 +1,7 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -import { UserInputError } from "../errors"; -import { apiPath } from "./api-path"; +import { UserInputError } from "./errors.js"; +import { apiPath } from "./api-path.js"; /** * Resolves a path the way `fetch` does. Dot-segment normalization happens during URL diff --git a/packages/mcp-core/src/api-client/api-path.ts b/packages/toolkit-core/src/api-client/api-path.ts similarity index 90% rename from packages/mcp-core/src/api-client/api-path.ts rename to packages/toolkit-core/src/api-client/api-path.ts index 1a85092e1..4443a751a 100644 --- a/packages/mcp-core/src/api-client/api-path.ts +++ b/packages/toolkit-core/src/api-client/api-path.ts @@ -1,5 +1,5 @@ -import { encodeApiPathSegment } from "@sentry/toolkit-core/api-path-segment"; -import { UserInputError } from "../errors"; +import { encodeApiPathSegment } from "../api-path-segment.js"; +import { UserInputError } from "./errors.js"; /** * Builds an API request path, confining each interpolated value to one path segment. diff --git a/packages/mcp-core/src/api-client/errors.ts b/packages/toolkit-core/src/api-client/errors.ts similarity index 93% rename from packages/mcp-core/src/api-client/errors.ts rename to packages/toolkit-core/src/api-client/errors.ts index a482aab0e..609fba61e 100644 --- a/packages/mcp-core/src/api-client/errors.ts +++ b/packages/toolkit-core/src/api-client/errors.ts @@ -1,3 +1,26 @@ +/** + * Error thrown when user input validation fails. + * These errors should be returned to the user directly without logging to Sentry. + */ +export class UserInputError extends Error { + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = "UserInputError"; + } +} + +/** + * Error thrown when configuration is invalid or missing. + * These errors should be returned to the user directly without logging to Sentry. + * Typically used for environment configuration issues, connection settings, etc. + */ +export class ConfigurationError extends Error { + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = "ConfigurationError"; + } +} + /** * API Error Class Hierarchy * From 4ddc5481329d3d75e186e1a10963bb3e132196f5 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:27:05 +0000 Subject: [PATCH 05/10] test(mcp): Keep the client.ts request-path guard next to client.ts The codemod moved api-path.test.ts into toolkit-core, but its second block reads mcp-core's client.ts source to check every request path uses apiPath. That block moves back to mcp-core until client.ts moves too. --- .../api-client/client-request-paths.test.ts | 71 +++++++++++++++++++ .../src/api-client/api-path.test.ts | 70 ------------------ 2 files changed, 71 insertions(+), 70 deletions(-) create mode 100644 packages/mcp-core/src/api-client/client-request-paths.test.ts diff --git a/packages/mcp-core/src/api-client/client-request-paths.test.ts b/packages/mcp-core/src/api-client/client-request-paths.test.ts new file mode 100644 index 000000000..c4b347c85 --- /dev/null +++ b/packages/mcp-core/src/api-client/client-request-paths.test.ts @@ -0,0 +1,71 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +/** + * The slug-only fix in VULN-848 hardened the parameters that were vulnerable at the + * time and left the resource IDs raw, which is how VULN-2159/2450 happened ten months + * later. Enumerating parameters does not hold; enforcing the invariant does. + * + * This fails on any new request path that interpolates a value without the `apiPath` + * tag, so the class cannot be reintroduced by a tool author unaware of it. + */ +describe("client.ts request path invariant", () => { + const source = readFileSync(new URL("./client.ts", import.meta.url), "utf8"); + + /** + * Returns the path portion of a template literal body, stopping at the query + * separator. Only interpolation before the `?` needs the tag; query values are + * already encoded by `URLSearchParams` and must keep their separators literal. + */ + function pathPortion(body: string): string { + let depth = 0; + for (let i = 0; i < body.length; i++) { + if (body[i] === "$" && body[i + 1] === "{") { + depth++; + i++; + } else if (body[i] === "}" && depth > 0) { + depth--; + } else if (body[i] === "?" && depth === 0) { + return body.slice(0, i); + } + } + return body; + } + + it("interpolates every request path through apiPath", () => { + const offenders: string[] = []; + + source.split("\n").forEach((line, index) => { + const trimmed = line.trim(); + if (trimmed.startsWith("//") || trimmed.startsWith("*")) return; + + // A template literal opening with a slash is an API request path. + for (let i = 0; i < line.length - 1; i++) { + if (line[i] !== "`" || line[i + 1] !== "/") continue; + + const closing = line.indexOf("`", i + 1); + const body = line.slice(i + 1, closing === -1 ? undefined : closing); + + if ( + !line.slice(0, i).endsWith("apiPath") && + /\$\{/.test(pathPortion(body)) + ) { + offenders.push(`${index + 1}: ${trimmed}`); + } + } + }); + + expect( + offenders, + `Request paths must be built with the apiPath tag so interpolated values cannot escape their segment:\n${offenders.join("\n")}`, + ).toEqual([]); + }); + + it("does not double-encode inside an apiPath template", () => { + const doubleEncoded = [...source.matchAll(/apiPath`[^`]*`/g)] + .map((match) => match[0]) + .filter((template) => template.includes("encodeURIComponent")); + + expect(doubleEncoded).toEqual([]); + }); +}); diff --git a/packages/toolkit-core/src/api-client/api-path.test.ts b/packages/toolkit-core/src/api-client/api-path.test.ts index 266c074f7..acd5a81cb 100644 --- a/packages/toolkit-core/src/api-client/api-path.test.ts +++ b/packages/toolkit-core/src/api-client/api-path.test.ts @@ -1,4 +1,3 @@ -import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; import { UserInputError } from "./errors.js"; import { apiPath } from "./api-path.js"; @@ -90,72 +89,3 @@ describe("apiPath", () => { }); }); }); - -/** - * The slug-only fix in VULN-848 hardened the parameters that were vulnerable at the - * time and left the resource IDs raw, which is how VULN-2159/2450 happened ten months - * later. Enumerating parameters does not hold; enforcing the invariant does. - * - * This fails on any new request path that interpolates a value without the `apiPath` - * tag, so the class cannot be reintroduced by a tool author unaware of it. - */ -describe("client.ts request path invariant", () => { - const source = readFileSync(new URL("./client.ts", import.meta.url), "utf8"); - - /** - * Returns the path portion of a template literal body, stopping at the query - * separator. Only interpolation before the `?` needs the tag; query values are - * already encoded by `URLSearchParams` and must keep their separators literal. - */ - function pathPortion(body: string): string { - let depth = 0; - for (let i = 0; i < body.length; i++) { - if (body[i] === "$" && body[i + 1] === "{") { - depth++; - i++; - } else if (body[i] === "}" && depth > 0) { - depth--; - } else if (body[i] === "?" && depth === 0) { - return body.slice(0, i); - } - } - return body; - } - - it("interpolates every request path through apiPath", () => { - const offenders: string[] = []; - - source.split("\n").forEach((line, index) => { - const trimmed = line.trim(); - if (trimmed.startsWith("//") || trimmed.startsWith("*")) return; - - // A template literal opening with a slash is an API request path. - for (let i = 0; i < line.length - 1; i++) { - if (line[i] !== "`" || line[i + 1] !== "/") continue; - - const closing = line.indexOf("`", i + 1); - const body = line.slice(i + 1, closing === -1 ? undefined : closing); - - if ( - !line.slice(0, i).endsWith("apiPath") && - /\$\{/.test(pathPortion(body)) - ) { - offenders.push(`${index + 1}: ${trimmed}`); - } - } - }); - - expect( - offenders, - `Request paths must be built with the apiPath tag so interpolated values cannot escape their segment:\n${offenders.join("\n")}`, - ).toEqual([]); - }); - - it("does not double-encode inside an apiPath template", () => { - const doubleEncoded = [...source.matchAll(/apiPath`[^`]*`/g)] - .map((match) => match[0]) - .filter((template) => template.includes("encodeURIComponent")); - - expect(doubleEncoded).toEqual([]); - }); -}); From 143da6e39a4396d83dd79965b374cd8378b39c06 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:55:48 +0000 Subject: [PATCH 06/10] feat(toolkit-core): Read Sentry API responses in the shared client readErrorResponse reads a failed response once: its text, parsed JSON, the server's string `detail`, whether it is an HTML page, and the rate-limit wait. Each host maps that to its own errors. readJsonResponse is the MCP's Content-Type-checked JSON parse, moved here for both. --- .../src/api-client/response.test.ts | 114 ++++++++++++++++++ .../toolkit-core/src/api-client/response.ts | 98 +++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 packages/toolkit-core/src/api-client/response.test.ts create mode 100644 packages/toolkit-core/src/api-client/response.ts diff --git a/packages/toolkit-core/src/api-client/response.test.ts b/packages/toolkit-core/src/api-client/response.test.ts new file mode 100644 index 000000000..ae4bb1223 --- /dev/null +++ b/packages/toolkit-core/src/api-client/response.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from "vitest"; +import { readErrorResponse, readJsonResponse } from "./response.js"; + +function jsonResponse( + body: unknown, + init: { status?: number; headers?: Record } = {}, +): Response { + return new Response(JSON.stringify(body), { + status: init.status, + headers: { "content-type": "application/json", ...init.headers }, + }); +} + +describe("readErrorResponse", () => { + it("reads the server's detail message", async () => { + const body = { detail: "Project not found", code: "missing" }; + const result = await readErrorResponse(jsonResponse(body, { status: 404 })); + expect(result).toEqual({ + text: JSON.stringify(body), + json: body, + detail: "Project not found", + isHtml: false, + retryAfterMs: undefined, + }); + }); + + it("leaves detail unset when it is not a string", async () => { + const result = await readErrorResponse( + jsonResponse({ detail: { code: "x" } }, { status: 400 }), + ); + expect(result.json).toEqual({ detail: { code: "x" } }); + expect(result.detail).toBeUndefined(); + }); + + it("flags an HTML error page", async () => { + const result = await readErrorResponse( + new Response("Bad gateway", { status: 502 }), + ); + expect(result.isHtml).toBe(true); + expect(result.json).toBeUndefined(); + expect(result.parseError).toBeTypeOf("string"); + }); + + it("keeps a plain-text body without flagging it as HTML", async () => { + const result = await readErrorResponse( + new Response("upstream timed out", { status: 504 }), + ); + expect(result).toMatchObject({ + text: "upstream timed out", + isHtml: false, + }); + expect(result.parseError).toBeTypeOf("string"); + }); + + it("reads the rate-limit wait from Retry-After", async () => { + const result = await readErrorResponse( + jsonResponse( + { detail: "Slow down" }, + { status: 429, headers: { "retry-after": "3" } }, + ), + ); + expect(result.retryAfterMs).toBe(3000); + }); + + it("returns no text when the body cannot be read", async () => { + const response = new Response("ignored", { status: 500 }); + await response.text(); + expect(await readErrorResponse(response)).toEqual({ + text: undefined, + isHtml: false, + retryAfterMs: undefined, + }); + }); +}); + +describe("readJsonResponse", () => { + it("parses a JSON body", async () => { + expect(await readJsonResponse(jsonResponse({ id: 1 }))).toEqual({ id: 1 }); + }); + + it("rejects an HTML body", async () => { + await expect( + readJsonResponse( + new Response("", { + status: 200, + statusText: "OK", + headers: { "content-type": "text/html" }, + }), + ), + ).rejects.toThrow( + "Expected JSON response but received HTML (200 OK). This may indicate you're not authenticated, the URL is incorrect, or there's a server issue.", + ); + }); + + it("rejects another content type", async () => { + await expect( + readJsonResponse( + new Response("id,1", { + status: 200, + statusText: "OK", + headers: { "content-type": "text/csv" }, + }), + ), + ).rejects.toThrow("Expected JSON response but received text/csv (200 OK)"); + }); + + it("rejects malformed JSON", async () => { + await expect( + readJsonResponse( + new Response("{", { headers: { "content-type": "application/json" } }), + ), + ).rejects.toThrow(/^Failed to parse JSON response: /); + }); +}); diff --git a/packages/toolkit-core/src/api-client/response.ts b/packages/toolkit-core/src/api-client/response.ts new file mode 100644 index 000000000..85c2e8b3f --- /dev/null +++ b/packages/toolkit-core/src/api-client/response.ts @@ -0,0 +1,98 @@ +/** + * Reading Sentry API responses. Hosts map what these return to their own + * errors: the MCP to `ApiError` subclasses, the CLI to its exit-coded ones. + */ + +import { parseRetryDelayMs } from "../api-retry.js"; + +/** What a failed response's body says, read once. */ +export type ErrorResponseBody = { + /** The raw body, or undefined when it could not be read. */ + text: string | undefined; + /** The parsed body, when it is JSON. */ + json?: unknown; + /** Why the body is not JSON, when it is not. */ + parseError?: string; + /** The server's `detail` message, when the JSON body has a string one. */ + detail?: string; + /** True when the body is an HTML page, usually from a proxy or a wrong URL. */ + isHtml: boolean; + /** Server-requested wait before retrying, from the rate-limit headers. */ + retryAfterMs?: number; +}; + +function readDetail(json: unknown): string | undefined { + if (json === null || typeof json !== "object" || !("detail" in json)) { + return undefined; + } + return typeof json.detail === "string" ? json.detail : undefined; +} + +function looksLikeHtml(text: string): boolean { + return text.includes(" { + const retryAfterMs = parseRetryDelayMs(response.status, response.headers); + let text: string; + try { + text = await response.text(); + } catch { + return { text: undefined, isHtml: false, retryAfterMs }; + } + try { + const json: unknown = JSON.parse(text); + return { + text, + json, + detail: readDetail(json), + isHtml: false, + retryAfterMs, + }; + } catch (error) { + return { + text, + parseError: error instanceof Error ? error.message : String(error), + isHtml: looksLikeHtml(text), + retryAfterMs, + }; + } +} + +/** + * Parse a successful response as JSON, checking its Content-Type first. + * + * @throws {Error} When the body is HTML, another content type, or malformed + */ +export async function readJsonResponse(response: Response): Promise { + // Test doubles and some promise chains pass responses without headers. + if (!response.headers?.get) { + return await response.json(); + } + + const contentType = response.headers.get("content-type"); + if (!contentType?.includes("application/json")) { + const text = await response.text(); + if (contentType?.includes("text/html") || looksLikeHtml(text)) { + // HTML in place of JSON usually means auth or routing went wrong. + throw new Error( + `Expected JSON response but received HTML (${response.status} ${response.statusText}). This may indicate you're not authenticated, the URL is incorrect, or there's a server issue.`, + ); + } + throw new Error( + `Expected JSON response but received ${contentType || "unknown content type"} ` + + `(${response.status} ${response.statusText})`, + ); + } + + try { + return await response.json(); + } catch (error) { + throw new Error( + `Failed to parse JSON response: ${error instanceof Error ? error.message : String(error)}`, + ); + } +} From b776eb55bde13028224b0b5bd152a8fb78674403 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:56:43 +0000 Subject: [PATCH 07/10] ref(mcp): Parse JSON responses with the shared reader Generated by the ts-morph codemod use-shared-json-response.mjs (script in the PR description), run from a scratch directory with ts-morph@28.0.0 installed: node use-shared-json-response.mjs oxfmt packages/mcp-core/src/api-client/client.ts It rewrites the 18 `this.parseJsonResponse(...)` calls in SentryApiService to toolkit-core's readJsonResponse, deletes the private method, and imports the shared one. --- packages/mcp-core/src/api-client/client.ts | 87 +++++----------------- 1 file changed, 19 insertions(+), 68 deletions(-) diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index c69d5661d..1339b3e61 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -215,6 +215,7 @@ import type { User, UserReportList, } from "./types"; +import { readJsonResponse } from "@sentry/toolkit-core/api-client/response"; // TODO: this is shared - so ideally, for safety, it uses @sentry/core, but currently // logger isnt exposed (or rather, it is, but its not the right logger) @@ -991,54 +992,6 @@ export class SentryApiService { return response; } - /** - * Safely parses a JSON response, checking Content-Type header first. - * - * @param response The Response object from fetch - * @returns Promise resolving to the parsed JSON object - * @throws {Error} If response is not JSON or parsing fails - */ - private async parseJsonResponse(response: Response): Promise { - // Handle case where response might not have all properties (e.g., in tests or promise chains) - if (!response.headers?.get) { - return await response.json(); - } - - const contentType = response.headers.get("content-type"); - - // Check if the response is JSON - if (!contentType || !contentType.includes("application/json")) { - const responseText = await response.text(); - - // Check if it's HTML - if ( - contentType?.includes("text/html") || - responseText.includes(" { const response = await this.request(path, options, requestOptions); - return await this.parseJsonResponse(response); + return await readJsonResponse(response); } /** Let SDK operations build paths and queries while MCP owns transport and errors. */ @@ -1089,7 +1042,7 @@ export class SentryApiService { ); const contentType = response.headers.get("content-type"); if (!contentType?.includes("application/json")) { - await this.parseJsonResponse(response); + await readJsonResponse(response); } return response; }, @@ -1956,7 +1909,7 @@ export class SentryApiService { undefined, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); return { dashboards: DashboardListSchema.parse(body), @@ -2162,7 +2115,7 @@ export class SentryApiService { { method: "GET" }, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); repos.push(...RepositoryListSchema.parse(body)); cursor = getNextCursor(response.headers.get("link")); } while (cursor); @@ -2256,7 +2209,7 @@ export class SentryApiService { opts, ); const rules = IssueAlertRuleListSchema.parse( - await this.parseJsonResponse(response), + await readJsonResponse(response), ); const targetLimit = limit ?? 100; return { @@ -2456,7 +2409,7 @@ export class SentryApiService { return { detectors: z .array(DetectorSchema) - .parse(await this.parseJsonResponse(response)), + .parse(await readJsonResponse(response)), nextCursor: getNextCursor(response.headers.get("link")), }; } @@ -2488,7 +2441,7 @@ export class SentryApiService { return { actions: z .array(AlertActionOptionSchema) - .parse(await this.parseJsonResponse(response)), + .parse(await readJsonResponse(response)), nextCursor: getNextCursor(response.headers.get("link")), }; } @@ -2520,7 +2473,7 @@ export class SentryApiService { return { conditions: z .array(AlertConditionOptionSchema) - .parse(await this.parseJsonResponse(response)), + .parse(await readJsonResponse(response)), nextCursor: getNextCursor(response.headers.get("link")), }; } @@ -2662,7 +2615,7 @@ export class SentryApiService { undefined, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); return { rules: MetricAlertRuleListSchema.parse(body), nextCursor: getNextCursor(response.headers.get("link")), @@ -2725,7 +2678,7 @@ export class SentryApiService { opts, ); return { - data: await this.parseJsonResponse(response), + data: await readJsonResponse(response), nextCursor: getNextCursor(response.headers.get("link")), }; } @@ -2765,7 +2718,7 @@ export class SentryApiService { undefined, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); teams.push(...TeamListSchema.parse(body)); cursor = getNextCursor(response.headers.get("link")); } while (cursor); @@ -3927,7 +3880,7 @@ export class SentryApiService { undefined, { ...opts, allowStatuses: [400] }, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); // ParseError responses use a detail envelope instead of validation results. // Keep malformed validation results reportable, even when their status is 400. if ( @@ -4212,7 +4165,7 @@ export class SentryApiService { undefined, opts, ); - return { data: await this.parseJsonResponse(response), response }; + return { data: await readJsonResponse(response), response }; }, schema); } @@ -4283,9 +4236,7 @@ export class SentryApiService { opts, ); return { - issue: NativeExternalIssueSchema.parse( - await this.parseJsonResponse(response), - ), + issue: NativeExternalIssueSchema.parse(await readJsonResponse(response)), changed: response.status === 201, }; } @@ -4400,7 +4351,7 @@ export class SentryApiService { { ...opts, host: this.isPublicSaas() ? "sentry.io" : opts?.host }, ); return { - issue: ExternalIssueSchema.parse(await this.parseJsonResponse(response)), + issue: ExternalIssueSchema.parse(await readJsonResponse(response)), changed: response.status === 201, }; } @@ -4456,7 +4407,7 @@ export class SentryApiService { undefined, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); return { reports: UserReportListSchema.parse(body), nextCursor: getNextCursor(response.headers.get("link")), @@ -5703,7 +5654,7 @@ export class SentryApiService { undefined, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); const details = AIConversationDetailsResponseSchema.parse(body); // Pages repeat the same conversation-scoped title. title = details.title; @@ -5785,7 +5736,7 @@ export class SentryApiService { undefined, opts, ); - const body = await this.parseJsonResponse(response); + const body = await readJsonResponse(response); return { conversations: AIConversationSummaryListSchema.parse(body), From 44976334d78eb9bdd197143d83d0ebc845379c33 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:59:31 +0000 Subject: [PATCH 08/10] ref(mcp): Read error responses with the shared reader SentryApiService.request reads a failed response with toolkit-core's readErrorResponse and maps it to ApiError subclasses as before: the same messages, details, retryAfterMs, and warnings. The Zod ApiErrorSchema it used for the `detail` check is gone. Two small changes: if the error body cannot be read, the caller now gets an ApiError for the status instead of the stream error, and the "no string detail" warning's validationErrorMessage is a fixed string instead of Zod's message. --- packages/mcp-core/src/api-client/client.ts | 165 ++++++++++----------- packages/mcp-core/src/api-client/schema.ts | 20 --- 2 files changed, 76 insertions(+), 109 deletions(-) diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index 1339b3e61..ded880082 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -21,10 +21,7 @@ import { } from "@sentry/api"; import { buildSentryApiUrl } from "@sentry/toolkit-core/api-request"; import { sentryBearerHeader } from "@sentry/toolkit-core/auth-token"; -import { - parseRetryDelayMs, - type RetryPolicy, -} from "@sentry/toolkit-core/api-retry"; +import type { RetryPolicy } from "@sentry/toolkit-core/api-retry"; import { createApiTransport } from "@sentry/toolkit-core/api-transport"; import { z } from "zod"; import { DEFAULT_SEARCH_ISSUES_PERIOD } from "../constants"; @@ -74,7 +71,6 @@ import { AlertActionOptionSchema, AlertConditionOptionSchema, AlertRuleProjectScopeSchema, - ApiErrorSchema, AutofixRunSchema, AutofixRunStateSchema, ClientKeyListSchema, @@ -215,7 +211,11 @@ import type { User, UserReportList, } from "./types"; -import { readJsonResponse } from "@sentry/toolkit-core/api-client/response"; +import { + type ErrorResponseBody, + readErrorResponse, + readJsonResponse, +} from "@sentry/toolkit-core/api-client/response"; // TODO: this is shared - so ideally, for safety, it uses @sentry/core, but currently // logger isnt exposed (or rather, it is, but its not the right logger) @@ -786,6 +786,72 @@ export class SentryApiService { } } + /** Map a failed response's body to the matching `ApiError` subclass. */ + private errorFromResponse( + response: Response, + body: ErrorResponseBody, + path: string, + ): ApiError { + const { retryAfterMs } = body; + const errorText = body.text ?? ""; + const logExtra = { + status: response.status, + statusText: response.statusText, + host: this.host, + path, + }; + const bodyPreview = + errorText.length > 256 ? `${errorText.slice(0, 253)}…` : errorText; + + if (body.parseError !== undefined) { + if (body.isHtml) { + logWarn("Received HTML error page instead of JSON", { + loggerScope: ["api", "client"], + extra: { ...logExtra, parseErrorMessage: body.parseError }, + }); + // HTML response instead of JSON typically indicates a server configuration issue + return createApiError( + `Server error: Received HTML instead of JSON (${response.status} ${response.statusText}). This may indicate an invalid URL or server issue.`, + response.status, + errorText, + undefined, + retryAfterMs, + ); + } + logWarn("Failed to parse JSON error response", { + loggerScope: ["api", "client"], + extra: { ...logExtra, bodyPreview, parseErrorMessage: body.parseError }, + }); + } else if (body.json) { + if (body.detail !== undefined) { + return createApiError( + body.detail, + response.status, + body.detail, + body.json, + retryAfterMs, + ); + } + logWarn("Failed to parse validated API error response", { + loggerScope: ["api", "client"], + extra: { + ...logExtra, + bodyPreview, + validationErrorMessage: "Error response has no string detail", + }, + }); + } + + // Use the error factory to create the appropriate error type based on status + return createApiError( + `API request failed: ${response.statusText}\n${errorText}`, + response.status, + errorText, + undefined, + retryAfterMs, + ); + } + /** * Makes an authenticated request to the Sentry API through the shared transport. * @@ -903,89 +969,10 @@ export class SentryApiService { if (allowStatuses?.includes(response.status)) { return response; } - const retryAfterMs = parseRetryDelayMs(response.status, response.headers); - const errorText = await response.text(); - let parsed: unknown; - try { - parsed = JSON.parse(errorText); - } catch (error) { - // If we can't parse JSON, check if it's HTML (server error) - if (errorText.includes(" 256 - ? `${errorText.slice(0, 253)}…` - : errorText, - parseErrorMessage: - error instanceof Error ? error.message : String(error), - }, - }); - } - - if (parsed) { - const { data, success, error } = ApiErrorSchema.safeParse(parsed); - - if (success) { - // Use the new error factory to create the appropriate error type - throw createApiError( - data.detail, - response.status, - data.detail, - parsed, - retryAfterMs, - ); - } - - logWarn("Failed to parse validated API error response", { - loggerScope: ["api", "client"], - extra: { - status: response.status, - statusText: response.statusText, - host: this.host, - path, - bodyPreview: - errorText.length > 256 - ? `${errorText.slice(0, 253)}…` - : errorText, - validationErrorMessage: - error instanceof Error ? error.message : String(error), - }, - }); - } - - // Use the error factory to create the appropriate error type based on status - throw createApiError( - `API request failed: ${response.statusText}\n${errorText}`, - response.status, - errorText, - undefined, - retryAfterMs, + throw this.errorFromResponse( + response, + await readErrorResponse(response), + path, ); } diff --git a/packages/mcp-core/src/api-client/schema.ts b/packages/mcp-core/src/api-client/schema.ts index 191e3df1f..7f058a8e8 100644 --- a/packages/mcp-core/src/api-client/schema.ts +++ b/packages/mcp-core/src/api-client/schema.ts @@ -26,14 +26,6 @@ * const issues = IssueListSchema.parse(await response.json()); * // TypeScript now knows the exact shape of issues * ``` - * - * @example Error Handling - * ```typescript - * const { data, success, error } = ApiErrorSchema.safeParse(response); - * if (success) { - * throw new ApiError(data.detail, statusCode); - * } - * ``` */ import type { SentryIssueIdentity, @@ -41,18 +33,6 @@ import type { } from "@sentry/toolkit-core/resource-identity"; import { z } from "zod"; -/** - * Schema for Sentry API error responses. - * - * Uses .passthrough() to allow additional fields that may be present - * in different error scenarios. - */ -export const ApiErrorSchema = z - .object({ - detail: z.string(), - }) - .passthrough(); - export const UserSchema = z .object({ id: z.union([z.string(), z.number()]), From 7027a482df1acc74a98210aa36169bd3cf1240a4 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:01:15 +0000 Subject: [PATCH 09/10] ref(cli): Read raw API error responses with the shared reader throwRawApiError takes the body, parsed JSON, and `detail` from toolkit-core's readErrorResponse instead of parsing them itself. The detail it puts on ApiError is unchanged: the server's string detail, the JSON body for statuses other than 401 and 403, the text for non-JSON bodies, and the status text when the body cannot be read. --- packages/cli/src/lib/api/infrastructure.ts | 30 ++++++++++------------ 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index c3573193d..3d138c684 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -13,6 +13,7 @@ import { type SdkResult, SentryApiError, } from "@sentry/api"; +import { readErrorResponse } from "@sentry/toolkit-core/api-client/response"; import { buildSentryApiUrl } from "@sentry/toolkit-core/api-request"; import { API_MAX_PER_PAGE, @@ -702,25 +703,20 @@ async function throwRawApiError( response: Response, endpoint: string, ): Promise { + const body = await readErrorResponse(response); let detail: string | undefined; - try { - const text = await response.text(); - try { - const parsed = JSON.parse(text) as { detail?: string }; - // Enriched statuses (403, 401) pass undefined when there is no - // usable string detail so the enrichment renders without a noisy - // `{"detail":null}` prefix. Other statuses get the full JSON as - // a debug aid. - if (typeof parsed.detail === "string") { - detail = parsed.detail; - } else if (response.status !== 403 && response.status !== 401) { - detail = JSON.stringify(parsed); - } - } catch { - detail = text || undefined; - } - } catch { + if (body.text === undefined) { detail = response.statusText; + } else if (body.parseError !== undefined) { + detail = body.text || undefined; + } else if (body.detail !== undefined) { + detail = body.detail; + } else if (response.status !== 403 && response.status !== 401) { + // Enriched statuses (403, 401) pass undefined when there is no + // usable string detail so the enrichment renders without a noisy + // `{"detail":null}` prefix. Other statuses get the full JSON as + // a debug aid. + detail = JSON.stringify(body.json); } // Attach a small allowlisted subset of response headers to the Sentry // event as context. This lets us distinguish Sentry-app 4xx/5xx (which From bf0b248ac75b43d8c4c4da20469cc318212c7b9f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:02:00 +0000 Subject: [PATCH 10/10] docs: Describe the shared API client modules The toolkit-core README covers the api-client modules and their portability guard, api-patterns.md says where ApiError and error reading live now, and the error-handling doc and logging skill point at the moved errors module. --- .agents/skills/logging-observability/SKILL.md | 4 ++-- docs/contributing/api-patterns.md | 7 ++++++ docs/contributing/error-handling.md | 2 +- packages/toolkit-core/README.md | 23 +++++++++++++++++++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/.agents/skills/logging-observability/SKILL.md b/.agents/skills/logging-observability/SKILL.md index e05d6c32c..30be9980d 100644 --- a/.agents/skills/logging-observability/SKILL.md +++ b/.agents/skills/logging-observability/SKILL.md @@ -9,7 +9,7 @@ Enforce correct logging and error handling patterns in the sentry-mcp codebase. ## Error Hierarchy -### API Errors (`packages/mcp-core/src/api-client/errors.ts`) +### API Errors (`packages/toolkit-core/src/api-client/errors.ts`) ``` ApiError (base class) @@ -299,7 +299,7 @@ When reviewing code for logging correctness, verify each item: ## Reference Files - Logging functions: `packages/mcp-core/src/telem/logging.ts` -- Error hierarchy: `packages/mcp-core/src/api-client/errors.ts` +- Error hierarchy: `packages/toolkit-core/src/api-client/errors.ts` - Error formatting: `packages/mcp-core/src/internal/error-handling.ts` - Agent tool wrapper: `packages/mcp-core/src/internal/agents/tools/utils.ts` - Full error handling guide: `docs/error-handling.md` diff --git a/docs/contributing/api-patterns.md b/docs/contributing/api-patterns.md index 34eee1a65..e22d86717 100644 --- a/docs/contributing/api-patterns.md +++ b/docs/contributing/api-patterns.md @@ -258,6 +258,13 @@ try { See error patterns in [common-patterns.md](common-patterns.md#error-handling). +`ApiError` and its subclasses live in the shared client, +`@sentry/toolkit-core/api-client/errors`, and `SentryApiService` builds them +from `readErrorResponse` in `@sentry/toolkit-core/api-client/response`. The +CLI reads raw error responses with the same helper and maps them to its own +exit-coded `ApiError`. See +[`packages/toolkit-core/README.md`](../../packages/toolkit-core/README.md#api-client). + ### Automatic Retries `SentryApiService` sends every request, including `@sentry/api` operations, diff --git a/docs/contributing/error-handling.md b/docs/contributing/error-handling.md index 8202ba6c2..9c87ff538 100644 --- a/docs/contributing/error-handling.md +++ b/docs/contributing/error-handling.md @@ -4,7 +4,7 @@ This document describes how errors are handled throughout the MCP server tool sy ## Error Types and Hierarchy -### API Error Classes (from api-client/errors.ts) +### API Error Classes (from `@sentry/toolkit-core/api-client/errors`) ``` ApiError (base class) diff --git a/packages/toolkit-core/README.md b/packages/toolkit-core/README.md index f3f94b631..15b139282 100644 --- a/packages/toolkit-core/README.md +++ b/packages/toolkit-core/README.md @@ -15,6 +15,29 @@ retains its own credential storage, regional cache identity and persistence, host allowlists, routing, polling deadline, response validation, and user-facing errors. +## API client + +`api-client/` is the shared Sentry API client, built here first and moved +into `@sentry/api` later, so it uses only web-standard APIs and runs in Node, +Workers, and browsers. Lint bans `node:*` imports and Node globals in +toolkit-core, and `pnpm run tsc` also typechecks it against the DOM library +with no Node types (`tsconfig.browser.json`). The `node:zlib` cache codec is +the one exception. + +- `errors` holds `ApiError` and its status subclasses, `createApiError`, and + the `UserInputError` and `ConfigurationError` the client throws. The MCP + re-exports them from `packages/mcp-core/src/errors.ts`. +- `api-path` builds API paths with every identifier segment encoded. +- `validate` checks a response with any + [Standard Schema](https://standardschema.dev) validator the host passes in: + Zod in the MCP, valibot in the CLI. Shared code depends only on the + interface's types. +- `response` reads responses. `readErrorResponse` reads a failed response's + text, parsed JSON, string `detail`, whether it is an HTML page, and the + rate-limit wait, and each host maps that to its own errors. + `readJsonResponse` parses a successful body after checking its + Content-Type. + ## API transport `api-transport` is the authenticated fetch both products send Sentry API