Make sure to consume HTTP error response bodies (#1173)

Co-authored-by: Eduardo Gomes <egomes@cloudflare.com>
Co-authored-by: Felix Weinberger <fweinberger@anthropic.com>
Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
This commit is contained in:
Eduardo Gomes 2025-11-26 16:52:56 +01:00 • committed by GitHub
commit 466483f25d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 39 additions and 13 deletions

View file

@ -625,10 +625,12 @@ export async function discoverOAuthProtectedResourceMetadata(
});
if (!response || response.status === 404) {
await response?.body?.cancel();
throw new Error(`Resource server does not implement OAuth 2.0 Protected Resource Metadata.`);
}
if (!response.ok) {
await response.body?.cancel();
throw new Error(`HTTP ${response.status} trying to load well-known OAuth protected resource metadata.`);
}
return OAuthProtectedResourceMetadataSchema.parse(await response.json());
@ -756,10 +758,12 @@ export async function discoverOAuthMetadata(
});
if (!response || response.status === 404) {
await response?.body?.cancel();
return undefined;
}
if (!response.ok) {
await response.body?.cancel();
throw new Error(`HTTP ${response.status} trying to load well-known OAuth metadata`);
}
@ -869,6 +873,7 @@ export async function discoverAuthorizationServerMetadata(
}
if (!response.ok) {
await response.body?.cancel();
// Continue looking for any 4xx response code.
if (response.status >= 400 && response.status < 500) {
continue; // Try next URL

View file

@ -253,6 +253,8 @@ export class SSEClientTransport implements Transport {
const response = await (this._fetch ?? fetch)(this._endpoint, init);
if (!response.ok) {
const text = await response.text().catch(() => null);
if (response.status === 401 && this._authProvider) {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
this._resourceMetadataUrl = resourceMetadataUrl;
@ -272,7 +274,6 @@ export class SSEClientTransport implements Transport {
return this.send(message);
}
const text = await response.text().catch(() => null);
throw new Error(`Error POSTing to endpoint (HTTP ${response.status}): ${text}`);
}
} catch (error) {

View file

@ -582,11 +582,13 @@ describe('StreamableHTTPClientTransport', () => {
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers()
headers: new Headers(),
text: async () => Promise.reject('dont read my body')
})
.mockResolvedValue({
ok: false,
status: 404
status: 404,
text: async () => Promise.reject('dont read my body')
});
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
@ -883,7 +885,8 @@ describe('StreamableHTTPClientTransport', () => {
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers()
headers: new Headers(),
text: async () => Promise.reject('dont read my body')
};
(global.fetch as Mock)
// Initial connection
@ -936,7 +939,8 @@ describe('StreamableHTTPClientTransport', () => {
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers()
headers: new Headers(),
text: async () => Promise.reject('dont read my body')
};
(global.fetch as Mock)
// Initial connection
@ -962,7 +966,8 @@ describe('StreamableHTTPClientTransport', () => {
// Fallback should fail to complete the flow
.mockResolvedValue({
ok: false,
status: 404
status: 404,
text: async () => Promise.reject('dont read my body')
});
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
@ -987,7 +992,8 @@ describe('StreamableHTTPClientTransport', () => {
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers()
headers: new Headers(),
text: async () => Promise.reject('dont read my body')
};
(global.fetch as Mock)
// Initial connection
@ -1013,7 +1019,8 @@ describe('StreamableHTTPClientTransport', () => {
// Fallback should fail to complete the flow
.mockResolvedValue({
ok: false,
status: 404
status: 404,
text: async () => Promise.reject('dont read my body')
});
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
@ -1026,7 +1033,8 @@ describe('StreamableHTTPClientTransport', () => {
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers()
headers: new Headers(),
text: async () => Promise.reject('dont read my body')
};
// Create custom fetch
@ -1328,7 +1336,8 @@ describe('StreamableHTTPClientTransport', () => {
ok: false,
status: 401,
statusText: 'Unauthorized',
headers: new Headers()
headers: new Headers(),
text: async () => Promise.reject('dont read my body')
};
(global.fetch as Mock)

View file

@ -223,6 +223,8 @@ export class StreamableHTTPClientTransport implements Transport {
});
if (!response.ok) {
await response.body?.cancel();
if (response.status === 401 && this._authProvider) {
// Need to authenticate
return await this._authThenStart();
@ -463,6 +465,8 @@ export class StreamableHTTPClientTransport implements Transport {
}
if (!response.ok) {
const text = await response.text().catch(() => null);
if (response.status === 401 && this._authProvider) {
// Prevent infinite recursion when server returns 401 after successful auth
if (this._hasCompletedAuthFlow) {
@ -525,7 +529,6 @@ export class StreamableHTTPClientTransport implements Transport {
}
}
const text = await response.text().catch(() => null);
throw new Error(`Error POSTing to endpoint (HTTP ${response.status}): ${text}`);
}
@ -535,6 +538,7 @@ export class StreamableHTTPClientTransport implements Transport {
// If the response is 202 Accepted, there's no body to process
if (response.status === 202) {
await response.body?.cancel();
// if the accepted notification is initialized, we start the SSE stream
// if it's supported by the server
if (isInitializedNotification(message)) {
@ -609,6 +613,7 @@ export class StreamableHTTPClientTransport implements Transport {
};
const response = await (this._fetch ?? fetch)(this._url, init);
await response.body?.cancel();
// We specifically handle 405 as a valid response according to the spec,
// meaning the server does not support explicit session termination

View file

@ -253,7 +253,8 @@ const tokenVerifier = {
});
if (!response.ok) {
throw new Error(`Invalid or expired token: ${await response.text()}`);
const text = await response.text().catch(() => null);
throw new Error(`Invalid or expired token: ${text}`);
}
const data = await response.json();

View file

@ -485,7 +485,8 @@ if (useOAuth) {
});
if (!response.ok) {
throw new Error(`Invalid or expired token: ${await response.text()}`);
const text = await response.text().catch(() => null);
throw new Error(`Invalid or expired token: ${text}`);
}
const data = await response.json();

View file

@ -84,6 +84,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
},
body: params.toString()
});
await response.body?.cancel();
if (!response.ok) {
throw new ServerError(`Token revocation failed: ${response.status}`);
@ -107,6 +108,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
});
if (!response.ok) {
await response.body?.cancel();
throw new ServerError(`Client registration failed: ${response.status}`);
}
@ -181,6 +183,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
});
if (!response.ok) {
await response.body?.cancel();
throw new ServerError(`Token exchange failed: ${response.status}`);
}
@ -221,6 +224,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
});
if (!response.ok) {
await response.body?.cancel();
throw new ServerError(`Token refresh failed: ${response.status}`);
}