fix(client/streamableHttp): retry sendMessage on 401#425
fix(client/streamableHttp): retry sendMessage on 401#425chrisdickinson wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
|
Whoops, looks like new tests landed since I wrote this commit; I'll update those. |
4917a3c to
40c1d84
Compare
|
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.
40c1d84 to
f4b357b
Compare
|
Thank you for working on this and very sorry about the delay in reviewing PRs. With the recent spec changes (now mcp servers are RS) and breaking compatibility in this PR, I would prefer to close this PR and keep the current retry logic on client. (The example in Closing this PR, but please feel free to open an issue/PR if you'd like to discuss alternative approaches to improve the OAuth flow experience while maintaining backward compatibility. |
…/pcarleton/naming rework naming on config copy
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
UnauthorizedErrorfrom 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'sredirectToAuthorizationcall. With this change in place, if client users calltransport.finishAuthbefore resolving the call toredirectToAuthorization, 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
catchexception handler, this change will no longer transfer control to their exception handler.Types of changes
Checklist