modelcontextprotocol / modelcontextprotocol/typescript-sdk

`maxTotalTimeout` is never enforced unless a progress notification arrives

Open
#2,695 10 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

v1 v2
Dominant language
TypeScript
Stars
13.4k
Forks
2.2k
Avg merge
3d 15h
Merged PRs (30d)
4

Description

What happens

maxTotalTimeout is documented as a hard cap:

Maximum total time (in milliseconds) to wait for a response. If exceeded, an SdkError with code SdkErrorCode.RequestTimeout will be raised, regardless of progress notifications.

It is not enforced unless a progress notification arrives and resetTimeoutOnProgress is set. Without both, the budget does nothing.

Measured on main, with a transport that never responds:

{ timeout: 1000, maxTotalTimeout: 150 }
  at 200ms  → still pending          (cap already exceeded)
  at 1200ms → "Request timed out"    (fired on `timeout`, not the cap)

{ maxTotalTimeout: 400 }
  after 3000ms → still pending       (waits for the 60s default timeout)

So a caller asking for a 150ms ceiling got 1000ms, and a caller asking for 400ms got 60s.

Why it happens

In packages/core-internal/src/shared/protocol.ts, _setupTimeout arms the pending timer with timeout only:

timeoutId: setTimeout(onTimeout, timeout),

maxTotalTimeout is read in exactly one place — the elapsed check inside _resetTimeout — and _resetTimeout has a single caller, guarded by:

if (timeoutInfo && responseHandler && timeoutInfo.resetTimeoutOnProgress) {

So the only way the budget is ever consulted is the progress path. A request with no progress notifications, or with resetTimeoutOnProgress left at its default of false, never checks it.

Why it survived

The existing test (test/shared/protocol.test.ts, "should respect maxTotalTimeout") passes resetTimeoutOnProgress: true and feeds two progress notifications, so it exercises the one path that works.

Expected

The pending timer should be armed for whichever limit comes first, and re-armed against the remaining budget on each progress notification, so the cap holds whether or not progress arrives.

Environment

main at 3924de99 (2.0.0-alpha.0). The same shape is present on v1.x.

Not related to #2224, #2136, #1870 or #2082 — those concern resetTimeoutOnProgress needing an onprogress callback, and none of them touch _setupTimeout or the arming.

I have a fix and tests ready and am happy to open a PR if useful.


AI assistance was used to investigate and write this report.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/core-internal/src/shared/protocol.ts, reading _setupTimeout and _resetTimeout and how the pending timer is armed. Run test/shared/protocol.test.ts, including the existing “should respect maxTotalTimeout” test. Done means the cap is enforced without progress notifications and remains correct when progress notifications reset the timeout.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.