Skip to content

fix: classify upstream errors correctly instead of returning 500#42

Merged
mattzcarey merged 2 commits intomainfrom
fix/error-classification
Feb 23, 2026
Merged

fix: classify upstream errors correctly instead of returning 500#42
mattzcarey merged 2 commits intomainfrom
fix/error-classification

Conversation

@mattzcarey
Copy link
Contributor

@mattzcarey mattzcarey commented Feb 23, 2026

Summary

Reports of frequent 5xx responses from the token and authorization endpoints revealed that upstream 4xx errors from Cloudflare's OAuth and API services were being swallowed as generic Error objects and returned as 500 Server Error pages. This violates the MCP auth spec which requires correct status codes:

Status Code Usage (per MCP spec)
401 Authorization required or token invalid
403 Invalid scopes or insufficient permissions
400 Malformed authorization request

This PR ports the same class of fixes from cloudflare/mcp-server-cloudflare#294 to the new MCP server.

Changes (4 files, +83 / -21)

src/auth/cloudflare-auth.ts

  • getAuthToken and refreshAuthToken now throw OAuthError with the upstream status code instead of generic Error
  • Added throwUpstreamError helper that maps upstream HTTP status to RFC-precise OAuth error codes:
    • 400 → invalid_grant, 401 → invalid_client, 403 → unauthorized_client, 429 → temporarily_unavailable, 5xx → server_error (502)
  • Raw upstream response bodies are logged via console.error but never sent to clients

src/auth/oauth-handler.ts

  • getUserAndAccounts now checks response.ok before parsing and throws OAuthError with RFC 6750 bearer token error codes:
    • 401 → invalid_token, 403 → insufficient_scope, 429 → temporarily_unavailable, 5xx → server_error (502)
  • Fallback throw changed from Error to OAuthError('invalid_token', ..., 401)
  • Fallback catch blocks in all 3 route handlers now return 500 (not 400) for truly unexpected errors
  • Added error correlation IDs (crypto.randomUUID()) to unexpected error responses — the ID appears in both the client-facing error page and console.error for log tracing
  • Removed e.message from client-facing error details to prevent information leakage

src/auth/workers-oauth-utils.ts

  • renderErrorPage accepts optional status parameter (default 400, backward-compatible)
  • OAuthError.toHtmlResponse() now forwards this.statusCode to renderErrorPage
  • Added invalid_grant, invalid_client, invalid_token, insufficient_scope to the HTML error title map
  • parseRedirectApproval throws OAuthError instead of generic Error for all validation failures:
    • Invalid method → 405, CSRF mismatch → 403, everything else → 400

src/auth/api-token-mode.ts

  • Catch block now checks for OAuthError and uses toResponse() (JSON) with the correct status code
  • Unknown errors still return 401 with a generic message (no e.message leakage)

What this does NOT change

  • executor.ts / server.ts — Tool execution errors already use MCP response format (isError: true)
  • No new error classes — reuses existing OAuthError
  • No changes to workers-oauth-provider

Test plan

  • npm run check passes (format, lint, typecheck, 69 tests)
  • Deploy to staging and test with curl
  • Verify: invalid token → 400 invalid_token (not 500)
  • Verify: missing state → 400 invalid_request (not 500)
  • Full OAuth flow with MCP Inspector
  • Verify error correlation IDs appear in worker logs

Upstream 4xx from Cloudflare OAuth and API endpoints were thrown as
generic Error objects, caught by route handlers, and returned as 500
Server Error pages. This violates the MCP auth spec which requires
401 for invalid tokens, 403 for insufficient permissions, and 400
for malformed requests.

Changes:
- Preserve upstream HTTP status codes (400/401/403/429) through
  OAuthError with RFC-precise error codes (invalid_grant,
  invalid_token, insufficient_scope, etc.)
- Map upstream 5xx to 502 Bad Gateway
- Replace generic Error throws in parseRedirectApproval with typed
  OAuthError instances
- Forward OAuthError statusCode through renderErrorPage and
  toHtmlResponse
- Add error correlation IDs to unexpected error responses for
  log tracing
- Remove raw error message leakage from client-facing responses
- Respect OAuthError status in API token mode catch block
@mattzcarey mattzcarey merged commit f3e938d into main Feb 23, 2026
4 checks passed
@mattzcarey mattzcarey deleted the fix/error-classification branch February 23, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant