Skip to content

fix(client/streamableHttp): retry sendMessage on 401 #425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chrisdickinson
Copy link
Contributor

After receiving a 401, attempt to auth. Whether the authorization works immediately or causes a redirect, retry sending the message.

Motivation and Context

I'm reading between the lines of the implementation a bit: without this change, client users have to catch the UnauthorizedError from the first 401 and attempt calling the client method again to re-submit the failed message if and only if they also received a call to their OAuth provider's redirectToAuthorization call. With this change in place, if client users call transport.finishAuth before resolving the call to redirectToAuthorization, message re-submission will be handled by the transport automatically.

I believe this matches the intent of the API, which is that the transport should retry after re-authorizing. However, I wanted to propose the behavior change – do we expect consumers of the client to drive re-submission after authorization, or should the transport handle that internally?

How Has This Been Tested?

I tested this using a jig that creates an MCP client and connects to an (in-development) server which acts as an oauth provider.

If this behavior change is desired, I can write additional tests.

Breaking Changes

If oauthProvider clients are currently relying on driving re-submission themselves via a catch exception handler, this change will no longer transfer control to their exception handler.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@chrisdickinson
Copy link
Contributor Author

Whoops, looks like new tests landed since I wrote this commit; I'll update those.

@chrisdickinson chrisdickinson force-pushed the chris/20250427-retry-on-401 branch from 4917a3c to 40c1d84 Compare April 29, 2025 18:15
@chrisdickinson
Copy link
Contributor Author

Okay, updated the test to reflect that the transport handles the entire oauth re-submission flow.

After receiving a 401, attempt to `auth`. Whether the authorization
works immediately or causes a redirect, retry sending the message.
@chrisdickinson chrisdickinson force-pushed the chris/20250427-retry-on-401 branch from 40c1d84 to f4b357b Compare April 29, 2025 22:34
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.

1 participant