-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: detect 401 errors from StreamableHTTP transport in client
#999
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
Conversation
|
Hi @cliffhall, can you help us take a look this pull request? Thank you. |
Hi @thinhtpt-dev! Thanks for this, and it does seem to solve the problem, but for Direct mode only. Still trying to figure out where Via Proxy is dropping the ball and hoping to get a single PR to fix it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged an extra check that fixes it for Via Proxy connection mode as well.
Adding fix for Via Proxy mode
cliffhall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works fine. I had to add an extra check that in the is401Error function that checks for "Missing Authorization header" which is what's coming back when it fails with Via Proxy connection type.
|
Tested against OAuth Servers in both modes, LGTM. |
|
@cliffhall I have tested the latest changes with my Keycloak and I can confirm that they have fixed the Direct mode. |
|
@cliffhall I have also tested the latest changes with https://example-server.modelcontextprotocol.io/mcp and I am getting different results. In particular, Proxy mode works fine, whereas in Direct mode, after the redirection and successful user authentication/authorization, the connection fails with the following generic error displayed under the Connect button: I've noticed, but I don't know if it is relevant, that in the Developers tools of the browser the following request returns a 400 Bad Request error: |
@jacopoc I'm looking into why the example-remote-server is failing at this. It may be because it has not had an SDK update. In the meantime, if you have the Typescript SDK checked out, you can do:
This will run a local server with OAuth. Then hit it with the Inspector at |
|
@cliffhall Following your notes, I have tested the latest version of the Inspector with the latest version of the simple server that is part of the v1.x branch of the Typescript SDK. |
@jacopoc woops, sorry about that. You have to open up CORS in import cors from "cors";
...
export function createMcpExpressApp(options: CreateMcpExpressAppOptions = {}): Express {
const { host = '127.0.0.1', allowedHosts } = options;
const app = express();
app.use(express.json());
app.use(
cors({
origin: "*", // use "*" with caution in production
methods: "GET,POST,DELETE",
preflightContinue: false,
optionsSuccessStatus: 204,
exposedHeaders: ["mcp-session-id", "last-event-id", "mcp-protocol-version"],
})
);
...
|
|
@cliffhall Thank you, I confirm it works with the test server. I am going to dig deeper to figure out the reason Proxy mode fails to work with my Keycloak instance and let you know if I find anything relevant for this project. |

Summary
Fix issue OAuth Flow was not triggered when using
StreamableHTTPwithDirectconnection type.Type of Change
Changes Made
Added
StreamableHTTPErrorhandling tois401Error()so 401 responses from the Streamable HTTP transport now trigger the existing auth-retry flow, keeping OAuth re-auth logic consistent across transport types.Related Issues
Fixes #982 and #960
Testing
Test Results and/or Instructions
How to test this fix:
Checklist
npm run prettier-fix)Breaking Changes
Additional Context