Skip to content

Commit fbf4943

Browse files
committed
Stop counting an interactive OAuth login against request timeouts
A 401 makes OAuthClientProvider await the developer's redirect_handler and callback_handler inside the challenged HTTP request, so a request's timeout also counted the person in the browser. For the connect-time server/discover probe that meant a login longer than its 10 seconds read as a legacy server, and Client(mode="auto") settled on 2025-11-25 against a server that speaks 2026-07-28. Any request with a read_timeout_seconds shorter than the login failed outright. send_raw_request now runs its timeout on a per-request clock held in a ContextVar, and the provider stops that clock while it awaits the two handlers. When they return, the timeout resumes with the budget that was left. The provider's own network calls stay on the clock, the probe keeps its 10 seconds, and a server that never answers is still given up on as before. A request timeout no longer ends a login nobody finishes: that takes a limit inside callback_handler or cancelling the caller. Fixes #3601
1 parent c15566c commit fbf4943

7 files changed

Lines changed: 538 additions & 13 deletions

File tree

‎src/mcp/client/auth/oauth2.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
validate_metadata_issuer,
4444
)
4545
from mcp.shared._httpx_utils import RedirectAwareAuth, redirect_note
46+
from mcp.shared._request_clock import waiting_on_a_person
4647
from mcp.shared.auth import (
4748
AuthorizationCodeResult,
4849
OAuthClientInformationFull,
@@ -425,10 +426,9 @@ async def _perform_authorization_code_grant(self) -> tuple[str, str]:
425426
auth_params["prompt"] = "consent"
426427

427428
authorization_url = f"{auth_endpoint}?{urlencode(auth_params)}"
428-
await self.context.redirect_handler(authorization_url)
429-
430-
# Wait for callback
431-
result = await self.context.callback_handler()
429+
with waiting_on_a_person():
430+
await self.context.redirect_handler(authorization_url)
431+
result = await self.context.callback_handler()
432432

433433
if result.state is None or not secrets.compare_digest(result.state, state):
434434
raise OAuthFlowError(f"State parameter mismatch: {result.state} != {state}")

‎src/mcp/shared/_request_clock.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
"""A request's timeout measures the peer, so its clock stops while the request waits on a person.
2+
3+
The clock travels in a `ContextVar`: the HTTP transports run each outgoing message in its sender's
4+
context, which is how code far below `send_raw_request` finds the clock of the request it serves.
5+
6+
Nothing in this module is public API: it may change or be removed without notice. It is likely to
7+
change with the client dispatcher work in https://github.com/modelcontextprotocol/python-sdk/pull/3517.
8+
"""
9+
10+
import math
11+
from collections.abc import Iterator
12+
from contextlib import contextmanager
13+
from contextvars import ContextVar
14+
15+
import anyio
16+
17+
18+
class RequestClock:
19+
"""One request's timeout, as a budget of seconds that is spent only while the clock runs.
20+
21+
Not public API: may change or be removed without notice.
22+
"""
23+
24+
def __init__(self, timeout: float | None, scope: anyio.CancelScope) -> None:
25+
self._budget = math.inf if timeout is None else timeout
26+
self._scope = scope
27+
self._pauses = 1 # the write, which is off the clock too; `start()` ends it
28+
29+
def start(self) -> None:
30+
self.resume()
31+
32+
def pause(self) -> None:
33+
if not self._pauses:
34+
self._budget = self._scope.deadline - anyio.current_time()
35+
self._scope.deadline = math.inf
36+
self._pauses += 1
37+
38+
def resume(self) -> None:
39+
self._pauses -= 1
40+
if not self._pauses:
41+
self._scope.deadline = anyio.current_time() + self._budget
42+
43+
44+
_clock: ContextVar[RequestClock | None] = ContextVar("request_clock", default=None)
45+
46+
47+
@contextmanager
48+
def request_clock(timeout: float | None) -> Iterator[RequestClock]:
49+
"""Put a clock, not yet started, in the context of the request sent in this block.
50+
51+
Raises `TimeoutError` once the clock has run for `timeout` seconds.
52+
53+
Not public API: may change or be removed without notice.
54+
"""
55+
with anyio.CancelScope() as scope:
56+
clock = RequestClock(timeout, scope)
57+
token = _clock.set(clock)
58+
try:
59+
yield clock
60+
finally:
61+
_clock.reset(token)
62+
# Not `fail_after`: it re-reads the deadline here, which a pause may have moved since it expired.
63+
if scope.cancelled_caught:
64+
raise TimeoutError
65+
66+
67+
@contextmanager
68+
def waiting_on_a_person() -> Iterator[None]:
69+
"""Stop the clock of the request this code is serving while the block is open; a no-op if there is none.
70+
71+
Not public API: may change or be removed without notice.
72+
"""
73+
clock = _clock.get()
74+
if clock is None:
75+
yield
76+
return
77+
clock.pause()
78+
try:
79+
yield
80+
finally:
81+
clock.resume()

‎src/mcp/shared/jsonrpc_dispatcher.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
from mcp.shared._compat import resync_tracer
4040
from mcp.shared._otel import inject_trace_context, otel_span
41+
from mcp.shared._request_clock import request_clock
4142
from mcp.shared._stream_protocols import ReadStream, WriteStream
4243
from mcp.shared.dispatcher import (
4344
CallOptions,
@@ -404,12 +405,13 @@ async def send_raw_request(
404405
# never started; past this point a cancelled write counts as issued.
405406
await anyio.lowlevel.checkpoint_if_cancelled()
406407
request_write_started = True
407-
try:
408-
await self._write(msg, plan.metadata)
409-
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
410-
# Transport tore down before run() noticed EOF; surface the documented contract.
411-
raise MCPError(code=CONNECTION_CLOSED, message="Connection closed") from None
412-
with anyio.fail_after(opts.get("timeout")):
408+
with request_clock(opts.get("timeout")) as clock:
409+
try:
410+
await self._write(msg, plan.metadata)
411+
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
412+
# Transport tore down before run() noticed EOF; surface the documented contract.
413+
raise MCPError(code=CONNECTION_CLOSED, message="Connection closed") from None
414+
clock.start()
413415
timeout_armed = True
414416
outcome = await receive.receive()
415417
if isinstance(outcome, ErrorData) and outcome is not _CLOSED_OUTCOME:
@@ -418,7 +420,7 @@ async def send_raw_request(
418420
span.set_status(StatusCode.ERROR, outcome.message)
419421
except TimeoutError:
420422
if not timeout_armed:
421-
# `fail_after` arms only after the write, so this TimeoutError is the
423+
# The clock starts only after the write, so this TimeoutError is the
422424
# transport's own bounded send() failing - a transport error, not
423425
# `opts["timeout"]` elapsing. Propagate it raw (v1 kept the write
424426
# outside the timeout-catching try and did the same).

‎tests/interaction/_requirements.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,15 @@ def __post_init__(self) -> None:
430430
),
431431
added_in="2026-07-28",
432432
),
433+
"lifecycle:discover:fallback-silence": Requirement(
434+
source=f"{SPEC_2026_BASE_URL}/basic/transports/stdio#backward-compatibility",
435+
behavior=(
436+
"When server/discover goes unanswered for the probe deadline, an auto-negotiating client falls "
437+
"back to the legacy initialize handshake."
438+
),
439+
added_in="2026-07-28",
440+
note="The spec states the timeout rule for stdio only; the SDK applies it on every transport.",
441+
),
433442
"lifecycle:discover:network-error-raises": Requirement(
434443
source="sdk",
435444
behavior=(
@@ -3826,6 +3835,25 @@ def __post_init__(self) -> None:
38263835
transports=("streamable-http",),
38273836
note="OAuth is HTTP-only.",
38283837
),
3838+
"client-auth:login-time:not-counted": Requirement(
3839+
source="issue:#3601",
3840+
behavior=(
3841+
"The time OAuthClientProvider spends awaiting redirect_handler and callback_handler does not count "
3842+
"against the timeout of the request that was challenged; afterwards the timeout resumes with the "
3843+
"budget that was left. The provider's own HTTP calls do count."
3844+
),
3845+
transports=("streamable-http",),
3846+
note="OAuth is HTTP-only.",
3847+
),
3848+
"client-auth:login-time:other-requests-keep-counting": Requirement(
3849+
source="sdk",
3850+
behavior=(
3851+
"A login suspends the timeout of the challenged request only: a request queued behind it in the "
3852+
"provider still times out on its own clock."
3853+
),
3854+
transports=("streamable-http",),
3855+
note="OAuth is HTTP-only.",
3856+
),
38293857
"client-auth:pkce:refuse-if-unsupported": Requirement(
38303858
source=f"{SPEC_BASE_URL}/basic/authorization#authorization-code-protection",
38313859
behavior=(

‎tests/interaction/auth/_harness.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,9 @@ async def connect_with_oauth(
418418
verify_tokens: bool = True,
419419
app_shim: Callable[[ASGIApp], ASGIApp] | None = None,
420420
on_request: Callable[[httpx2.Request], None] | None = None,
421+
mode: str = "legacy",
422+
read_timeout_seconds: float | None = None,
423+
json_response: bool = False,
421424
) -> AsyncIterator[tuple[Client, HeadlessOAuth]]:
422425
"""Connect a `Client` to a server's bearer-gated streamable-HTTP app, completing OAuth in process.
423426
@@ -455,6 +458,7 @@ async def connect_with_oauth(
455458
)
456459

457460
app: ASGIApp = server.streamable_http_app(
461+
json_response=json_response,
458462
auth=settings,
459463
token_verifier=ProviderTokenVerifier(provider) if verify_tokens else None,
460464
auth_server_provider=provider,
@@ -481,7 +485,11 @@ async def hook(request: httpx2.Request) -> None:
481485
)
482486
headless.bind(http_client)
483487
client = await stack.enter_async_context(
484-
# The auth flow tests snapshot the legacy initialize-handshake HTTP shape.
485-
Client(streamable_http_client(f"{BASE_URL}/mcp", http_client=http_client), mode="legacy")
488+
# The auth flow tests snapshot the legacy initialize-handshake HTTP shape, hence the default mode.
489+
Client(
490+
streamable_http_client(f"{BASE_URL}/mcp", http_client=http_client),
491+
mode=mode,
492+
read_timeout_seconds=read_timeout_seconds,
493+
)
486494
)
487495
yield client, headless

0 commit comments

Comments
 (0)