Fix server/discover timeout interrupting OAuth#1719
Conversation
Start the short server/discover timeout only after the request has been sent so OAuth authentication remains governed by the overall initialization timeout. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@PranavSenthilnathan I'm also supportive of increasing the test timeouts, but I think it's a functional issue to have the probe timeout affected by the OAuth flow. I think it makes sense for the initialize timeout to still be in force, but how long the OAuth flow takes shouldn't affect protocol negotiation. |
| var probeTimeout = _options.DiscoverProbeTimeout; | ||
| using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(initializationCts.Token); | ||
| if (_options.InitializationTimeout > probeTimeout) | ||
| { | ||
| probeCts.CancelAfter(probeTimeout); | ||
| } | ||
| var discoverResponseTimeout = _options.InitializationTimeout > probeTimeout | ||
| ? probeTimeout | ||
| : Timeout.InfiniteTimeSpan; |
There was a problem hiding this comment.
Can we move these closer to the use site in the local function?
|
|
||
| // Start the response timeout after sending completes so transport setup and authentication remain | ||
| // governed by the caller's cancellation token rather than consuming the response budget. | ||
| using var responseTimeoutCts = responseTimeout == Timeout.InfiniteTimeSpan |
There was a problem hiding this comment.
This fixes DiscoverProbeTimeout prematurely canceling OAuth, but it no longer bounds the Streamable HTTP response wait. SendToRelatedTransportAsync awaits and processes the POST response before returning, so the timeout starts only after that response has been read and queued. If the server never responds, the client waits for InitializationTimeout instead of falling back after DiscoverProbeTimeout.
Passing the probe token into the entire send would reintroduce the OAuth bug because that operation may receive a 401, perform separate OAuth requests, and retry the POST internally. The HTTP/OAuth layer likely needs to apply the probe timeout while awaiting each MCP POST response, exclude the OAuth requests, and use a fresh timeout for the authenticated retry, while InitializationTimeout remains active throughout.
AI-assisted review comment.
Summary
Start
DiscoverProbeTimeoutonly after theserver/discoverrequest has been sent. This keeps transport setup and OAuth authorization under the overallInitializationTimeout, while preserving the short response timeout used to fall back when an older server silently dropsserver/discover.The regression test now deliberately keeps protected-resource metadata discovery in flight longer than the probe timeout and verifies that the fallback paths are requested exactly once.
Main CI failure
This fixes the failure in Build and Test run 29739945453, Ubuntu Debug job 88344129696:
The logs show the initial OAuth flow beginning for
server/discover, then the five-second probe timeout startinginitializebefore the first token exchange completed:That overlap launched a second OAuth flow and repeated protected-resource metadata discovery.
Tests
dotnet build --no-restoreModelContextProtocol.Testssuite on net9.0 (2,286 passed)