Skip to content

Fix Gateway Error Propagation & Frontend JSON Parse Issues#2654

Merged
crivetimihai merged 1 commit intomainfrom
fix_gateway_registration_error_propogation
Feb 7, 2026
Merged

Fix Gateway Error Propagation & Frontend JSON Parse Issues#2654
crivetimihai merged 1 commit intomainfrom
fix_gateway_registration_error_propogation

Conversation

@kevalmahajan
Copy link
Copy Markdown
Member

@kevalmahajan kevalmahajan commented Feb 2, 2026 •
edited
Loading

🐛 Bug-fix PR

Closes #2562

📌 Summary

This PR addresses two issues:

  1. Backend Error Propagation: Fixes an issue where specific backend errors (e.g., 401 Unauthorized or 405 Method Not Allowed) were masked by generic "Failed to initialize gateway" messages. This prevented users from seeing the actual cause of gateway registration failures allowing users to find the root cause of failure and fix it, instead of relying on backend logs.

  2. Frontend JSON Parsing: Fixes a frontend silent crash/error with ("JSON.parse: unexpected character") that occurred when the server or a proxy returned an HTML response (e.g., error pages, auth redirects) instead of the expected JSON.
    Root Cause documented here: [BUG]: JSON parse error when adding MCP server - missing response validation in admin.js #2562 (comment).

🔁 Reproduction Steps

1. Backend Error Propagation:

  • Attempt to register a gateway with invalid credentials (causing a 401).
  • Before: Error message in UI/logs: Failed to initialize gateway....
  • After: Error message in UI/logs: Failed to initialize gateway... Client error '401 Unauthorized'.

2. Frontend JSON Parse Error:

  • Trigger an HTML response for a form submission endpoint (e.g., POST /admin/gateways) by simulating a proxy error (502/504) or session expiry redirect.
  • Before: The UI crashes or shows a console error: SyntaxError: JSON.parse: unexpected character at line 1 column 1 because the code tried to parse <html>... as JSON.
  • After: The UI correctly handles the non-JSON response and displays a user-friendly error message: "The server returned an unexpected response..."

🐞 Root Cause

Backend:

  • The MCP SDK uses anyio.TaskGroup for concurrency, which wraps raised exceptions in a BaseExceptionGroup.
  • The existing error handling code simply called str(e) on the caught exception, which returned the wrapper's generic message instead of the inner exception's detail.

Frontend:

  • Multiple form handlers in admin.js directly awaited response.json() without validating the response status or Content-Type.
  • If an upstream proxy or the server returned an HTML error page (common in enterprise variants or during outages), response.json() would fail with a syntax error, breaking the error handling flow.

💡 Fix Description

Backend (gateway_service.py):

  • Updated _initialize_gateway to inspect caught exceptions.
  • Implemented logic to unwrap BaseExceptionGroup and extract the root cause exception (matching patterns used elsewhere in tool_service.py).
  • Ensures the actual HTTP error detail is propagated to the GatewayConnectionError.

Frontend (admin.js):

  • Created a reusable helper function safeParseJsonResponse(response, fallbackError) that:
    1. Checks response.ok status first.
    2. Validates that the Content-Type header includes result "application/json".
    3. Throws a descriptive error if the response is not valid JSON, preventing the parse crash.
  • Refactored 12 high-risk form handlers (POST/PUT operations) to use this safe helper instead of direct response.json():
    • Gateway (Add & Edit)
    • Resource (Add & Edit)
    • Prompt (Add & Edit)
    • Server (Add & Edit)
    • A2A Agent (Add & Edit)
    • Tool (Add & Edit)
      Note: There can be more occurrences where the safeParseJsonResponse can be used, but currently have targeted to the high impact ones in this PR.

🧪 Verification

Check Command Status
Lint suite make lint
Unit tests make test
Coverage ≥ 80 % make coverage
Manual regression no longer fails steps / screenshots

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

@crivetimihai crivetimihai changed the title Fix Gateway Error Propagation & Frontend JSON Parse Vulnerabilities Fix Gateway Error Propagation & Frontend JSON Parse Issues Feb 3, 2026
@crivetimihai crivetimihai added this to the Release 1.0.0-RC1 milestone Feb 3, 2026
@crivetimihai
Copy link
Copy Markdown
Member

crivetimihai commented Feb 4, 2026

Well-documented fix with clear root cause analysis. The BaseExceptionGroup unwrapping matches the pattern already used in tool_service.py, and the safeParseJsonResponse helper is a solid defensive pattern for the frontend form handlers. CI is all green.

LGTM — ready to merge.

@crivetimihai crivetimihai self-assigned this Feb 4, 2026
@crivetimihai crivetimihai force-pushed the fix_gateway_registration_error_propogation branch 3 times, most recently from 46c7ff4 to d06418e Compare February 7, 2026 00:55
@crivetimihai
Copy link
Copy Markdown
Member

crivetimihai commented Feb 7, 2026

Code Review Follow-up

Addressed additional security and UX improvements during review:

✅ Fixed

  • Userinfo redaction: sanitize_url_for_logging() now redacts user:pass@host credentials in URLs as defense-in-depth
  • HTML error handling: parseErrorResponse() detects HTML responses and shows user-friendly message instead of raw HTML; truncates long text to 200 chars
  • Consistent error formatting: Both register_gateway and update_gateway now use str(ex) directly
  • Test coverage: Added message assertion to update_gateway test

⏭️ Not Addressed (by design)

Item Reason
Machine-readable error codes (e.g., upstream_status field) Out of scope - would require API schema changes and client updates
ExceptionGroup "first exception wins" Follows established pattern used in 3 other places in tool_service.py (lines 3219-3221, 3366-3368, 3579-3581)
application/problem+json Content-Type Not used anywhere in codebase
204 No Content handling Admin form endpoints don't return 204

These could be considered for future enhancements if needed.

@kevalmahajan @crivetimihai
fix: propagate gateway errors and handle non-JSON responses in frontend
f33a13f 
- Backend: Extract root cause from BaseExceptionGroup when MCP SDK uses
  TaskGroup, ensuring actual HTTP errors (401, 405, etc.) are shown
  instead of generic "Failed to initialize gateway" messages
- Backend: Change HTTP status from 503 to 502 for GatewayConnectionError
  as 502 Bad Gateway more accurately represents upstream server failures
- Backend: Include sanitized error details in GatewayConnectionError
  messages for better debugging while protecting sensitive URL params
- Backend: Add userinfo (user:pass@host) redaction to sanitize_url_for_logging
  as defense-in-depth against credential leakage in error messages
- Frontend: Add safeParseJsonResponse() helper to validate response
  status and Content-Type before parsing JSON, preventing crashes when
  proxies or auth redirects return HTML error pages
- Frontend: Update extractApiError() to also check error.message field
- Frontend: Detect HTML responses and show user-friendly message instead
  of raw HTML; truncate long text responses to 200 chars
- Apply safeParseJsonResponse to 12 high-risk form handlers (POST/PUT):
  Gateway, Resource, Prompt, Server, A2A Agent, Tool (add & edit each)

Closes #2562

Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai force-pushed the fix_gateway_registration_error_propogation branch from d06418e to f33a13f Compare February 7, 2026 00:58
@crivetimihai crivetimihai merged commit a071e7f into main Feb 7, 2026
51 checks passed
@crivetimihai crivetimihai deleted the fix_gateway_registration_error_propogation branch February 7, 2026 01:09
@crivetimihai crivetimihai added the ica ICA related issues label Feb 11, 2026
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
@kevalmahajan
fix: propagate gateway errors and handle non-JSON responses in fronte…
5f2740b 
…nd (IBM#2654)

- Backend: Extract root cause from BaseExceptionGroup when MCP SDK uses
  TaskGroup, ensuring actual HTTP errors (401, 405, etc.) are shown
  instead of generic "Failed to initialize gateway" messages
- Backend: Change HTTP status from 503 to 502 for GatewayConnectionError
  as 502 Bad Gateway more accurately represents upstream server failures
- Backend: Include sanitized error details in GatewayConnectionError
  messages for better debugging while protecting sensitive URL params
- Backend: Add userinfo (user:pass@host) redaction to sanitize_url_for_logging
  as defense-in-depth against credential leakage in error messages
- Frontend: Add safeParseJsonResponse() helper to validate response
  status and Content-Type before parsing JSON, preventing crashes when
  proxies or auth redirects return HTML error pages
- Frontend: Update extractApiError() to also check error.message field
- Frontend: Detect HTML responses and show user-friendly message instead
  of raw HTML; truncate long text responses to 200 chars
- Apply safeParseJsonResponse to 12 high-risk form handlers (POST/PUT):
  Gateway, Resource, Prompt, Server, A2A Agent, Tool (add & edit each)

Closes IBM#2562

Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@madhav165 madhav165 mentioned this pull request Feb 27, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crivetimihai crivetimihai crivetimihai approved these changes

@madhav165 madhav165 Awaiting requested review from madhav165 madhav165 is a code owner

Assignees

@crivetimihai crivetimihai

Labels

ica ICA related issues

Projects

None yet

Milestone

Release 1.0.0-RC1

Development

Successfully merging this pull request may close these issues.

[BUG]: JSON parse error when adding MCP server - missing response validation in admin.js

2 participants

@kevalmahajan @crivetimihai