Repository navigation
ref(mcp): Send Sentry API requests through the shared transport - #1722
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b3e98f3. Configure here.
Add `fetch` to McpServerOptions and ServerContext. SentryApiService sends every request through it, including @sentry/api operations, so a host can supply its authenticated transport. Without one, requests go through the shared @sentry/toolkit-core/api-transport with REQUEST_RETRY_POLICY. SentryApiService drops its own retry loop, so retries happen in one layer and do not stack. ApiRateLimitError still takes retryAfterMs from the final response's headers, and an abort during a retry wait keeps the caller's reason. The transport's credentials and timeouts become optional: without a CredentialProvider it keeps the request's Authorization header, never refreshes, and skips the cache. Implements #1669 task 6.1. Co-Authored-By: BYK <burak.kaya@sentry.io>
The transport aborted its per-attempt controller without the caller's reason, so an in-flight fetch rejected with a generic AbortError. Forward signal.reason, and treat a rejection with that reason as the caller's abort rather than a retryable network failure. Co-Authored-By: BYK <burak.kaya@sentry.io>
The shared API layer moves into toolkit-core and then @sentry/api, so the MCP no longer needs a host fetch hook. SentryApiService keeps sending every request through the shared transport with no retry loop of its own. Co-Authored-By: BYK <burak.kaya@sentry.io>
sentry-junior
Bot
force-pushed
the
feat/mcp-transport-injection
branch
from
October 29, 2026 16:36
3e4d8d0 to
0a3ca36
Compare
sentry-junior
Bot
changed the title
BYK
approved these changes
Oct 29, 2026
BYK
pushed a commit
that referenced
this pull request
Oct 29, 2026
…se reading (#1732) Starts the shared Sentry API client in `toolkit-core/src/api-client/` (step 1 of #1669, task group 8). The MCP and the CLI both use it for errors, API paths, and response reading. It is the first of a stack of PRs by layer (see "Next" below). ## What changes - **Portability guard (8.1).** Lint bans `node:*` imports and Node globals such as `Buffer` and `process` in `toolkit-core`. A second typecheck (`tsconfig.browser.json`) checks it against the DOM library with no Node types, and `pnpm run tsc` runs both. The `node:zlib` cache codec is the one exception. Its types move to `response-cache/body.ts`, so the policy and store modules stay portable. - **Standard Schema validation.** `validateResponse` takes any Standard Schema validator: Zod from the MCP, valibot from the CLI. `toolkit-core` depends only on `@standard-schema/spec` types. Lint bans `zod` and `valibot` imports in its source; they are dev dependencies for the tests only. - **Shared errors and `apiPath`.** `ApiError`, its status subclasses, `createApiError`, `UserInputError`, `ConfigurationError`, and `apiPath` move from `mcp-core` to `@sentry/toolkit-core/api-client/*`. `mcp-core/src/errors.ts` re-exports the two general errors, so its importers don't change. - **Shared response reading.** `readErrorResponse` reads a failed response once: its text, parsed JSON, string `detail`, whether it is an HTML page, and the rate-limit wait. `readJsonResponse` is the MCP's Content-Type-checked JSON parse. - `SentryApiService` uses both. - The CLI's `throwRawApiError` uses `readErrorResponse`. - Each product still maps the result to its own errors (MCP `ApiError` subclasses, CLI exit-coded `ApiError`). - **mcp-core builds ESM only.** Once the errors moved, the CJS build hit a rolldown "canonical name not found" panic. Nothing loads that CJS output: - `@sentry/mcp-core` is `private` and `"type": "module"`. - All 29 of its exports point at `.js`, with no `require` condition. - Every consumer is in the workspace: `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 ESM packages. - No source requires it or reads its `.cjs` files. `@sentry/mcp-server`'s own CJS and ESM output is unchanged. I tried upgrading tsdown first, to 0.15.12, 0.18.4, 0.20.3, 0.22.14, and 0.23.4. Every version broke the `mcp-server-mocks` build: 0.15.12 failed declaration output (`members is not iterable`), and 0.18.4 and later hit a rolldown JSON parse "recursion limit exceeded" on `src/fixtures/trace.json`. 0.18.4 and 0.20.3 also failed the `mcp-core` and `mcp-server` builds. Bundling toolkit-core into the dist (`noExternal`) would have duplicated the error classes across dists and broken `instanceof` between packages. - **Docs.** The `toolkit-core` README now has an "API client" section. `api-patterns.md`, `error-handling.md`, and the logging skill point at the new error module. ## Behavior changes - **MCP, unreadable error body.** If the body of a failed response cannot be read, the caller now gets an `ApiError` for the status instead of the raw stream error. - **MCP, "no string detail" warning.** The `validationErrorMessage` on the "Failed to parse validated API error response" warning is now a fixed string instead of Zod's message. `ApiErrorSchema` is deleted; nothing else used it. Messages, details, `retryAfterMs`, and the CLI's error detail are otherwise unchanged. ## Next This is the bottom of a stack of PRs by layer for the rest of #1669 group 8. Each one is based on the PR below it: 1. Move pagination, cache policy, retry, and transport under `api-client/`. 2. Shared endpoint wrappers on top of `@sentry/api`, with `SentryApiService` switched onto them. 3. The CLI host seam, with the CLI's `lib/api/` wrappers switched onto the shared client. 4. Merge the wrappers both products share. This PR does not touch `packages/mcp-server`, `packages/cli/src/lib/mcp.ts`, or the evals stdio entry; the `@sentry/mcp-server` retirement PR owns those. ## Commits Each codemod run is a generated commit whose message gives the command, and each hand fix is a separate commit: 1. `13053adab` Portability guard (hand) 2. `f13df0d24` Standard Schema validation (hand) 3. `bea21d648` mcp-core ESM only (hand) 4. `0d083bdf3` Move errors and `apiPath` (**generated**, `move-shared-errors.mjs`) 5. `4ddc54813` Move the `client.ts` request-path test back next to `client.ts` (hand) 6. `143da6e39` Shared response reading (hand) 7. `b776eb55b` MCP JSON parsing on the shared reader (**generated**, `use-shared-json-response.mjs`) 8. `44976334d` MCP error responses on the shared reader (hand) 9. `7027a482d` CLI raw error responses on the shared reader (hand) 10. `bf0b248ac` Docs (hand) ## Codemods The scripts are not in the tree. To re-run one, check out the parent of its generated commit, run the script from a scratch directory with `ts-morph@28.0.0` installed, then run oxfmt on the changed files. The output should match the commit. <details> <summary><code>move-shared-errors.mjs</code> (commit 0d083bd)</summary> ```sh node move-shared-errors.mjs <repo-root> oxfmt <added and modified files> ``` ```js // Codemod: move the MCP's API error classes and API path helper into // toolkit-core's shared api-client, and rewrite every importer. // // Usage: node move-shared-errors.mjs <toolkit-repo-root> // (run from a directory where `ts-morph@28.0.0` is installed) import { realpathSync } from "node:fs"; import path from "node:path"; import { Project } from "ts-morph"; const root = path.resolve(process.argv[2] ?? "."); const mcpSrc = path.join(root, "packages/mcp-core/src"); const coreSrc = path.join(root, "packages/toolkit-core/src"); // Classes the shared client throws, moved out of mcp-core's general errors. const MOVED_CLASSES = ["UserInputError", "ConfigurationError"]; // mcp-core files that move into toolkit-core/src/api-client/. const MOVED_FILES = [ "api-client/errors.ts", "api-client/api-path.ts", "api-client/api-path.test.ts", ]; const project = new Project({ tsConfigFilePath: path.join(root, "packages/mcp-core/tsconfig.json"), }); project.addSourceFilesAtPaths(path.join(coreSrc, "**/*.ts")); // 1. Move the classes into the API errors module; mcp-core/src/errors.ts // re-exports them so its existing importers keep working. const generalErrors = project.getSourceFileOrThrow( path.join(mcpSrc, "errors.ts"), ); const apiErrors = project.getSourceFileOrThrow( path.join(mcpSrc, "api-client/errors.ts"), ); const classTexts = MOVED_CLASSES.map((name) => { const declaration = generalErrors.getClassOrThrow(name); const text = declaration.getFullText().trim(); declaration.remove(); return text; }); apiErrors.insertText(0, `${classTexts.join("\n\n")}\n\n`); generalErrors.insertExportDeclaration(0, { namedExports: [...MOVED_CLASSES].sort(), moduleSpecifier: "./api-client/errors", }); // 2. Move the files. ts-morph rewrites relative imports in and to them. const movedPaths = new Set(); for (const file of MOVED_FILES) { const sourceFile = project.getSourceFileOrThrow(path.join(mcpSrc, file)); sourceFile.move(path.join(coreSrc, "api-client", path.basename(file))); movedPaths.add(sourceFile.getFilePath()); } // Moved files take the moved classes from the API errors module directly, // not through mcp-core's re-export. for (const movedPath of movedPaths) { const sourceFile = project.getSourceFileOrThrow(movedPath); for (const declaration of sourceFile.getImportDeclarations()) { if (declaration.getModuleSpecifierSourceFile() !== generalErrors) continue; const names = declaration.getNamedImports().map((n) => n.getName()); if (names.every((name) => MOVED_CLASSES.includes(name))) { declaration.setModuleSpecifier(apiErrors); } } } // 3. Normalize specifiers that point into toolkit-core: relative with `.js` // inside toolkit-core, `@sentry/toolkit-core/<path>` everywhere else. function realPath(file) { try { return realpathSync(file); } catch { return file; } } for (const sourceFile of project.getSourceFiles()) { const importer = sourceFile.getFilePath(); const declarations = [ ...sourceFile.getImportDeclarations(), ...sourceFile.getExportDeclarations(), ]; for (const declaration of declarations) { const target = declaration.getModuleSpecifierSourceFile(); if (!target) continue; const targetPath = realPath(target.getFilePath()); if (!targetPath.startsWith(`${coreSrc}/`)) continue; // Leave specifiers the codemod did not affect as they are. if (!movedPaths.has(importer) && !movedPaths.has(targetPath)) continue; const withoutExt = targetPath.replace(/\.ts$/, ""); let specifier; if (importer.startsWith(`${coreSrc}/`)) { specifier = path.relative(path.dirname(importer), withoutExt); if (!specifier.startsWith(".")) specifier = `./${specifier}`; specifier = `${specifier}.js`; } else { specifier = `@sentry/toolkit-core/${path .relative(coreSrc, withoutExt) .replace(/\/index$/, "")}`; } if (declaration.getModuleSpecifierValue() !== specifier) { declaration.setModuleSpecifier(specifier); } } } await project.save(); ``` </details> <details> <summary><code>use-shared-json-response.mjs</code> (commit b776eb5)</summary> ```sh node use-shared-json-response.mjs <repo-root> oxfmt packages/mcp-core/src/api-client/client.ts ``` ```js // Codemod: replace SentryApiService's private parseJsonResponse with the // shared readJsonResponse from toolkit-core, and delete the private copy. // // Usage: node use-shared-json-response.mjs <toolkit-repo-root> // (run from a directory where `ts-morph@28.0.0` is installed) import path from "node:path"; import { Project, SyntaxKind } from "ts-morph"; const root = path.resolve(process.argv[2] ?? "."); const clientPath = path.join(root, "packages/mcp-core/src/api-client/client.ts"); const project = new Project({ skipAddingFilesFromTsConfig: true }); const client = project.addSourceFileAtPath(clientPath); const service = client.getClassOrThrow("SentryApiService"); // 1. `this.parseJsonResponse(x)` -> `readJsonResponse(x)`. const calls = client .getDescendantsOfKind(SyntaxKind.CallExpression) .filter((call) => call.getExpression().getText() === "this.parseJsonResponse") .reverse(); for (const call of calls) { call.getExpression().replaceWithText("readJsonResponse"); } // 2. Delete the private method it replaces. service.getMethodOrThrow("parseJsonResponse").remove(); // 3. Import the shared helper. client.addImportDeclaration({ namedImports: ["readJsonResponse"], moduleSpecifier: "@sentry/toolkit-core/api-client/response", }); await project.save(); console.log(`rewrote ${calls.length} call sites`); ``` </details> ## Verification At `bf0b248ac`, rebased on `main` after #1722 merged: - `pnpm run tsc` passes. It includes toolkit-core's browser-target typecheck and the CLI typecheck. - `pnpm run lint` passes (oxlint, oxfmt, ast-grep rules), and so does `pnpm run docs:check`. - Tests: toolkit-core 306, mcp-core 1881 (6 skipped), and mcp-cloudflare 443 pass. - Before the rebase, the full `pnpm run test` also passed mcp-server (87), mcp-test-client (80), docs-mcp (49), local (322), and cli-docs. - CLI (`test/lib`, `test/commands`): 11718 pass and 6 fail in 5 files (`completions`, `dsn/code-scanner`, `init/tools/file-changes`, `telemetry`, `upgrade`). - All 6 test permission errors (EACCES/EPERM/read-only). They fail only because the sandbox runs with `CAP_DAC_OVERRIDE`, so a `chmod 000` file stays readable. - This PR touches none of that code; CI is the check for them. - Both generated commits reproduce exactly when their codemod is re-run on the rebased parent. - The diff has no codemod scripts, ast-grep or ts-morph config, or ts-morph dependency. <!-- junior-request-attribution:start --> via **BYK**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3Aa6eddc0161cff61914454da9f74fcdf1 --> -- [View Junior Session in Sentry](https://sentry.sentry.io/explore/conversations/agent%3Aa6eddc0161cff61914454da9f74fcdf1/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR sends every MCP request to the Sentry API through the shared transport, and moves retries out of
SentryApiServiceinto it. It is task 6.1 of #1669 (unify-sentry-api-layer) and builds on the shared transport (#1719) and the fresh-read hook (#1720).What changed
SentryApiServicesends every request throughcreateApiTransportfrom@sentry/toolkit-core/api-transport, using the MCP's existingREQUEST_RETRY_POLICY. That includes@sentry/apioperations, which already go throughrequest().SentryApiServiceno longer has its own retry loop or network-failure bookkeeping. The shared transport retries a 429 for any method, and a 500/502/503/504 or a network failure for GET and HEAD.SentryApiServicestill parses the final error response, soApiRateLimitError.retryAfterMsis still read fromRetry-AfterorX-Sentry-Rate-Limit-Reset.credentialsandtimeoutMsoncreateApiTransportare now optional. Without aCredentialProvider, the transport keeps theAuthorizationheader the request already has, never refreshes, and skips the cache, because cache entries are tied to a credential. The MCP uses it this way, and still sets no timeout, as before.signal.reason, instead of being treated as a retryable network failure.An earlier revision also added a host-supplied
fetchoption toMcpServerOptionsandServerContext. That is gone: per the revised #1669 plan, the shared API layer moves into toolkit-core and then@sentry/api, and the CLI passing its fetch tosentry mcpis deferred.Intentional behavior changes
allowStatuses: a status a caller lists there is now returned only after the transport has finished retrying it. Before, it came back on the first attempt. Today's callers only list 404 and 400, which are never retried, so they see no difference.Relation to #1669
SentryApiServiceand the CLI's API modules into one browser-safe API client in toolkit-core, stacked on this PR.Verification
New
mcp-coretests:maxRetries + 1times.New
toolkit-coretests:Authorizationis kept.Suites:
toolkit-coremcp-coremcp-servermcp-cloudflaretsc --noEmitpasses fortoolkit-core,mcp-core,mcp-server,mcp-cloudflare, andcli.oxlintandoxfmt --checkare clean on the changed files.Test changes:
client.test.tsgained astatus, because the transport reads it to decide on a retry.client.test.tsandmcp-handler.test.tsnow read throughHeaders, because the transport passes aHeadersobject.via BYK.
--
View Junior Session in Sentry