Skip to content

[Client] Throw TimeoutException on request timeout and cancel the request - #567

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix-client-request-timeout
Oct 12, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix-client-request-timeout

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

The default request timeout used `time()`, so it fired up to a second early, and it surfaced as a generic `RequestException` (-32603) without telling the server.

  • the default timeout now runs as a deadline like `callTool(timeoutSeconds:)` => sub-second, `TimeoutException` and `notifications/cancelled`
  • `TimeoutException` extends `RequestException`, so existing catchers keep working
  • the init timeout stays error-based (probe fallback depends on it), but with `microtime()` resolution

Fixes chr-hertel#68
Fixes chr-hertel#69

🤖 Generated with Claude Code

@chr-hertel chr-hertel added this to the 0.9.0 milestone Oct 10, 2026
@chr-hertel chr-hertel added bug Something isn't working Client Issues & PRs related to the Client component labels Oct 10, 2026
@chr-hertel
chr-hertel requested a balanced review from Copilot October 12, 2026 20:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The changelog does not disclose the public pending-request timestamp type compatibility change.

1 open finding
What changed in this PR

Improves request-timeout precision and surfaces timeouts consistently as cancellable TimeoutExceptions.

Changes:

  • Uses microsecond-resolution timestamps.
  • Applies default timeouts as deadlines and sends cancellation signals.
  • Updates exception inheritance, tests, documentation, and changelog.
File Description
src/​Client.php Applies the configured timeout as a deadline.
src/​Client/​State/​ClientState.php Stores precise request timestamps.
src/​Client/​State/​ClientStateInterface.php Updates the pending-request timestamp type.
src/​Client/​Transport/​HttpTransport.php Uses precise timeout comparisons.
src/​Client/​Transport/​StdioTransport.php Uses precise timeout comparisons.
src/​Exception/​TimeoutException.php Makes timeouts request exceptions.
tests/​Integration/​CancellationTest.php Tests default timeout timing and cancellation.
docs/​client/​errors.md Documents timeout handling.
docs/​client/​transports.md Documents timeout cancellation behavior.
CHANGELOG.md Records the timeout fix.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread CHANGELOG.md
…uest

The default request timeout now runs as a sub-second deadline like the per-call one.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Timeout documentation contradicts existing tables and inaccurately claims modern HTTP sends cancellation notifications.

3 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread CHANGELOG.md
Comment thread docs/client/errors.md
Comment thread docs/client/transports.md
@chr-hertel
chr-hertel merged commit 91f7089 into modelcontextprotocol:main Oct 12, 2026
28 checks passed
@chr-hertel
chr-hertel deleted the fix-client-request-timeout branch October 12, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Client Issues & PRs related to the Client component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Client] Request timeout isn't a TimeoutException and sends no cancellation [Client] Request timeout only has 1-second resolution

2 participants