fix(mcp): explain 404 upstream dials and bump go-sdk to v1.8.0 - #987
SantiagoDePolonia wants to merge 1 commit into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if err == nil && req.Method != http.MethodGet && | ||
| (resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusMethodNotAllowed) { | ||
| p.status.Store(int32(resp.StatusCode)) | ||
| } | ||
| return resp, err | ||
| } | ||
|
|
||
| // missingEndpoint returns the recorded status, or 0 when every POST of the | ||
| // dial reached an MCP endpoint. | ||
| func (p *connectProbe) missingEndpoint() int { | ||
| if p == nil { | ||
| return 0 | ||
| } | ||
| return int(p.status.Load()) | ||
| } | ||
|
|
||
| // connectError wraps a failed dial, naming the URL path when the probe saw | ||
| // that nothing MCP is served there. | ||
| func (u *upstream) connectError(err error, probe *connectProbe) error { | ||
| if status := probe.missingEndpoint(); status != 0 { | ||
| return fmt.Errorf("connect to mcp server %q: %s answered HTTP %d %s; no MCP endpoint at that path, check the url: %w", | ||
| u.spec.Name, endpointForLog(u.spec.URL), status, http.StatusText(status), err) | ||
| } |
There was a problem hiding this comment.
If a stateful MCP server loses its session after initialize succeeds, the later notifications/initialized POST can return 404. This probe records that later response and replaces the session-specific failure with “no MCP endpoint at that path,” even though the configured endpoint already returned 200 during initialization. Operators may change a correct URL instead of addressing the expired session. This is a non-blocking diagnostic accuracy concern.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
- Focused in-process Go test that accepts initialization and returns 404 after the stateful session disappears, demonstrating the incorrect diagnostic.
- Output from the prior revision showing the session-specific error without the endpoint-path diagnostic.
- Output from the changed revision showing successful initialization followed by the incorrect missing-endpoint diagnostic.
| parsed.User = nil | ||
| parsed.RawQuery = "" | ||
| parsed.Fragment = "" | ||
| return parsed.String() |
There was a problem hiding this comment.
endpointForLog removes userinfo, query, and fragment values but preserves the complete URL path. If an upstream URL contains a credential in its path, a 404 or 405 connection failure includes that credential in the connection error and dashboard-facing LastError. This violates the repository directive never to expose secrets in errors or logs and can disclose an endpoint token to people who can view diagnostics.
How this was verified: A 404 and a 405 response each produced an error and dashboard value containing the configured path credential.
Context Used: AGENTS.md (source)
Artifacts
- The authored in-process Go test configures a path credential and exercises 404 and 405 upstream failures, with assertions for error and dashboard propagation; it defines the exact reproduction.
- The executed Go test output records exit code 0 and shows the path credential in each 404/405 connect error and dashboard LastError; the finding is confirmed.
1 participant
Fixes #986.
Stateless MCP servers (mcp-devtools v2, Firecrawl) already connect through the gateway; verified against a go-sdk stateless server and the live Firecrawl endpoint. The reported
failed to connect (session ID: ): session not foundis go-sdk v1.7.0 labelling a plain HTTP 404 from the configured URL, which happens when theurllacks the server's endpoint path (/httpfor mcp-devtools,/v2/mcpfor Firecrawl, both changed recently).User-visible impact:
subscriptions/listenfor stateless servers, fixesClient.Connectleaks and asubscriptionsListendeadlock, and lets 2026-07-28 clients renegotiate against our stateful/mcphandler instead of getting a plain 400.<url> answered HTTP 404 Not Found; no MCP endpoint at that path, check the urlin logs and the dashboard's last error. Userinfo and query are stripped from the URL.