Skip to content

Conversation

@arnewouters
Copy link
Contributor

@arnewouters arnewouters commented Nov 26, 2025

Summary

Changes

Please provide a summary of what's being changed

Timeouts stop the background session task in FastMCP, this makes follow-up tool calls fail with

MCP Tool Error Response: Internal error: nesting counter should be 0 when starting new session, got 2

With this PR, we are explicitly refreshing the session when the client is no longer connected.

User experience

Please share what the user experience looks like before and after this change

config for testing:

    "example-mcp-server": {
      "command": "uv",
        "args": [
          "run",
          "--directory",
          "/Volumes/workplace/mcp-proxy-for-aws-dev",
          "mcp-proxy-for-aws",
          "--service",
          "my-service",
          "--region",
          "us-east-1",
          "--log-level",
          "DEBUG",
          "https://example.com/mcp"
        ],
        "env": {},
        "disabled": false,
        "autoApprove": []
    }

Cline

Before:

There is an error after a timeout

Untitled Diagram drawio

After:

Session refreshes under the hood and the next call succeeds

Untitled Diagram2 drawio

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • I have reviewed the contributing guidelines
  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented

Is this a breaking change? (Y/N)

  • Yes
  • No

Please add details about how this change was tested.

  • Did integration tests succeed?
  • If the feature is a new use case, is it necessary to add a new integration test case?

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

args.endpoint, service, region, metadata, timeout, profile
)

async with _initialize_client(transport) as client:
Copy link
Contributor

Choose a reason for hiding this comment

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

The client should be a ProxyClient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Because we probably want a mix of the if-else branches

  1. connect the session to be reused
  2. use ProxyClient to support features like elicitation and sampling; the regular client does not support

@arnewouters arnewouters marked this pull request as ready for review November 27, 2025 09:54
@arnewouters arnewouters requested a review from a team as a code owner November 27, 2025 09:54
nonlocal client
if not client.is_connected():
logger.debug('Reinitialize client')
client = ProxyClient(transport)
Copy link
Contributor

Choose a reason for hiding this comment

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

what if you do client.new() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It returns Client and not ProxyClient

if not client.is_connected():
logger.debug('Reinitialize client')
client = ProxyClient(transport)
await client._connect()
Copy link
Contributor

@wzxxing wzxxing Nov 27, 2025

Choose a reason for hiding this comment

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

When this method throws, the client also hangs, the _initialize_client context manager writes the jsonrpc error to stdout.

Can this be done here too? I think it is a bit tricky to get the new json rpc message id.

@wzxxing wzxxing merged commit c7ff533 into aws:main Nov 27, 2025
3 checks passed
@arnewouters arnewouters deleted the fix/refresh-session-after-timeout branch November 27, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants