Skip to content

fix: handle non-success HTTP responses in MCP gallery query (fixes #321963)#321966

Open
vs-code-engineering[bot] wants to merge 1 commit into
mainfrom
fix/mcp-gallery-404-321963-441eaae0fc6f94a8
Open

fix: handle non-success HTTP responses in MCP gallery query (fixes #321963)#321966
vs-code-engineering[bot] wants to merge 1 commit into
mainfrom
fix/mcp-gallery-404-321963-441eaae0fc6f94a8

Conversation

@vs-code-engineering

Copy link
Copy Markdown
Contributor

Summary

Telemetry bucket 8c52e78a-53e7-b4d1-4111-f24a79ae8cda (unhandlederrorError: Server returned 404) fires when the MCP gallery server responds to a server-listing query with a non-success HTTP status. In McpGalleryService.queryRawGalleryMcpServers, the response is passed to asJson(context) with no status-code guard and outside the surrounding network-error try/catch. asJson throws Server returned <status> for any non-2xx response, and because that call sits outside the guarded path the throw escapes as an unhandled error into telemetry.

The sibling method getMcpServer in the same file already guards 4xx responses before calling asJson; queryRawGalleryMcpServers was missing the equivalent guard. The fix adds a guard clause that covers all non-success statuses (4xx and 5xx), logs the failure, and degrades gracefully to an empty result — matching the existing network-error handling in the same method.

Observed at version 1.125.0 (shipped commit 93cfdd489c3b228840d0f86ec77c3636277c93ea): 423 hits / 180 users, first seen 2026-06-17.

Fixes #321963
Recommended reviewer: @sandy081

Culprit Commit

Most relevant commit: 1a2b823d90ed7eebf6f03b5be2525d61ef4fbe66 (2026-05-14) — fix: handle network errors in MCP gallery query gracefully (fixes #315094) (#315096), authored by the release automation account vs-code-engineering[bot] (co-authored by Copilot and bryanchen-d). It wrapped requestService.request(...) in a try/catch to gracefully handle network/transport failures, but left the response-body parse (asJson) outside that guarded path and added no HTTP status-code check. As a result a response that is successfully received but carries a non-2xx status (e.g. 404) still reaches asJson and throws.

Classification: the unhandled-non-success path is effectively pre-existing / latentasJson has always thrown on non-success status. The bucket appears only at 1.125.0 (single release, first seen 2026-06-17), which points to a server- or manifest-side change as the proximate spike trigger: the query URL is resolved from the gallery manifest via getMcpGalleryManifestResourceUri(...), which is external to this repository. The commit above is the closest contributing change in this repo because it established graceful degradation for this exact method but stopped short of covering HTTP status failures.

Code Flow

flowchart TD
    A["McpGalleryService.query / getMcpServersFromGallery"] --> B["queryRawGalleryMcpServers()"]
    B --> C["requestService.request(GET gallery query URL)"]
    C -->|network/transport error| D["catch: logService.error + return empty result"]
    C -->|HTTP response received| E["context (res.statusCode = 404)"]
    E --> F["await asJson(context)"]
    F --> G{"isSuccess(context)?"}
    G -->|no| H["throw new Error('Server returned 404')"]
    H --> I["unhandled error → telemetry bucket 8c52e78a..."]
    G -->|yes| J["parse JSON body → serialize result"]
    E -. fix adds guard here .-> K["if (!isSuccess(context)) → logService.error + return empty result"]
Loading

Affected Files

  • src/vs/platform/mcp/common/mcpGalleryService.ts
    • Added isSuccess to the import from ../../request/common/request.js.
    • Added a guard clause in queryRawGalleryMcpServers (immediately after the request try/catch, before await asJson(context)) that returns an empty result and logs when the response is not a success status.

Repro Steps

  1. Point VS Code at an MCP gallery whose server-query endpoint returns a non-2xx status (e.g. 404 — the gallery resource/manifest endpoint is temporarily unavailable, moved, or returns not-found).
  2. Trigger an MCP gallery server-listing query (e.g. browse/search MCP servers, or any flow that calls IMcpGalleryService.queryqueryRawGalleryMcpServers).
  3. The request resolves with a 404 response; queryRawGalleryMcpServers calls asJson(context), which throws Error: Server returned 404 because the status is not a success status.
  4. The throw is outside the network-error try/catch, so it propagates as an unhandled error and is recorded in telemetry (bucket 8c52e78a-53e7-b4d1-4111-f24a79ae8cda).

How the Fix Works

Chosen approach (src/vs/platform/mcp/common/mcpGalleryService.ts): Insert a guard clause immediately after the request try/catch and before const data = await asJson(context); (the producer site that consumes the unguarded response):

if (!isSuccess(context)) {
    this.logService.error(`Failed to query MCP gallery: Server returned ${context.res.statusCode}`);
    return { servers: [], metadata: { count: 0 } };
}

isSuccess is the exact predicate asJson evaluates internally before deciding to throw, so checking it at the producer guarantees the throw is never reached. On a non-success response the method now logs via logService.error (preserving the diagnostic signal rather than silencing it) and returns the same empty result ({ servers: [], metadata: { count: 0 } }) the method already returns for network errors and empty payloads. This places the fix at the data producer that consumes the response — not at the shared crash site in request.ts (the data-producer principle) — and uses a guard clause rather than a try/catch that would mask the error from telemetry (the prefer-guard-clauses principle). It also brings the method in line with the sibling getMcpServer, which already guards before calling asJson.

Alternatives considered:

  • Guard only 4xx (as getMcpServer does): rejected because 5xx gallery responses would still throw the same unhandled error; !isSuccess covers all non-2xx, matching exactly the condition asJson throws on.
  • Wrap asJson in a try/catch: rejected because it would hide the failure from the diagnostic/telemetry path instead of preventing the throw at the producer, violating the no-silencing principle.
  • Modify asJson / request.ts (the crash site): rejected — that is the shared crash site used by many callers, and changing its throw contract would mask legitimate failures elsewhere (do not fix at the crash site).

Recommended Owner

@sandy081 — owns MCP (MCP Gallery, MCP Management) and Extensions Management per the team working-areas registry, and authored nearly all recent commits to mcpGalleryService.ts (actively maintained, most recent commits dated 2026-06-18). The culprit commit's author is the release automation account vs-code-engineering[bot], which is not a routable human owner, so ownership falls to the feature-area maintainer.

Generated by errors-fix · 1.8K AIC · ⌖ 88.6 AIC · ⊞ 69.3K ·

…21963)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 16:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@vs-code-engineering vs-code-engineering Bot marked this pull request as ready for review June 18, 2026 16:58
@vs-code-engineering vs-code-engineering Bot enabled auto-merge (squash) June 18, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Error] unhandlederror-Server returned 404

2 participants