fix(httpclient): treat HTML responses as AuthRequiredErr in readResponse#293
Open
mmailhos wants to merge 1 commit into
Open
fix(httpclient): treat HTML responses as AuthRequiredErr in readResponse#293mmailhos wants to merge 1 commit into
mmailhos wants to merge 1 commit into
Conversation
When an upstream MCP server's OAuth token expires, the server may return an HTML login-redirect page (status 200 or 3xx-followed) instead of JSON. readResponse currently returns a generic "invalid response format" error for any non-JSON body, which is not caught by the AuthRequiredErr or oauth2: handlers in Send(), so the error surfaces as a confusing "failed to decode response: invalid character '<'" instead of triggering the OAuth re-authentication flow. Fix: before reading the body, check Content-Type for text/html and return AuthRequiredErr. As a defence-in-depth measure, also check if the body starts with '<' after ReadAll, which catches cases where the Content-Type header is missing but the body is clearly HTML. This makes the OAuth re-authentication flow trigger correctly when a stored upstream token expires mid-session (e.g. Amplitude MCP). Made-with: Cursor
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.
Problem
When an upstream MCP server's OAuth access token expires, the server may return an HTML login-redirect page (HTTP 200 after following a 3xx, or the redirect body itself) instead of a JSON response.
readResponsecurrently returns a generic"invalid response format"error for any non-{body. This error is not caught by either theAuthRequiredErroroauth2:handlers inSend(), so instead of triggering the OAuth re-authentication flow, the error surfaces to the caller as:Root cause
Send()has two recovery paths for auth failures:errors.AsType[AuthRequiredErr](err)— deletes the stored token and re-runs the OAuth flowstrings.HasPrefix(err, "oauth2:")— resets tohttp.DefaultClientand retries (which triggers path 1)Neither matches the plain
"invalid response format"string returned byreadResponsefor HTML bodies, so both paths are skipped and the error propagates as-is.Fix
Before reading the body:
Content-Type: text/html→ returnAuthRequiredErrimmediately (discarding the body)After
ReadAll:<, the body is almost certainly HTML from an auth redirect → returnAuthRequiredErrThis makes
Send()correctly trigger the OAuth re-authentication flow when a stored upstream token expires.Reproduction
Observed with Amplitude MCP (
https://mcp.amplitude.com/mcp) used as a remote server behind the Obot gateway. After the initial OAuth connection, the stored Amplitude access token expires. On the next MCP request, Amplitude responds with an HTML redirect to its login page. With this fix,readResponsereturnsAuthRequiredErr,Send()deletes the stale token and re-runs the OAuth flow transparently.Testing
go build ./pkg/mcp/...passes"failed to decode response: invalid character '<' looking for beginning of value"disappears after applying the fixMade with Cursor