Return auth errors #451
Return auth errors #451
Conversation
github-actions
bot
added
T-core
jamadeo
force-pushed
the
jamadeo/error-pass-thru
branch
from
fb01267 to
7d7f11b
Compare
| Err(err) => { | ||
| let _ = responder.send(Err(err)); | ||
| return Err(WorkerQuitReason::fatal( | ||
| StreamableHttpError::TransportChannelClosed, |
There was a problem hiding this comment.
this is the only bit I don't understand. why do we only return this here? are the other variants of StreamableHttpError not relevant?
There was a problem hiding this comment.
This is an unfortunate result of the fact that you can't generally clone an Err. But it doesn't matter too much here: the returned error here is logged/thrown away when .close is called on the transport here: https://github.com/modelcontextprotocol/rust-sdk/blob/3310718b9c922a618007649eeaf9bc3d9a53fe80/crates/rmcp/src/transport/worker.rs#L208. The important part is the line before, where we .send() it to the caller.
That said I could put an actual message here, which would improve it
There was a problem hiding this comment.
All good. Makes sense!
jamadeo
force-pushed
the
jamadeo/error-pass-thru
branch
from
7d7f11b to
3f460a7
Compare
When streamable HTTP servers return 401s, surface them to the client.
Motivation and Context
This is necessary for clients to determine whether oauth is needed.
How Has This Been Tested?
Tested with https://github.com/block/goose
Breaking Changes
It adds a new error variant to StreamableHttpError
Types of changes
Checklist
Additional context