Skip to content

Commit 466483f

Browse files
GreenStageEduardo Gomesfelixweinberger
authored
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>
1 parent e343bf5 commit 466483f

File tree

7 files changed

+39
-13
lines changed

7 files changed

+39
-13
lines changed

‎src/client/auth.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,10 +625,12 @@ export async function discoverOAuthProtectedResourceMetadata(
625625
});
626626

627627
if (!response || response.status === 404) {
628+
await response?.body?.cancel();
628629
throw new Error(`Resource server does not implement OAuth 2.0 Protected Resource Metadata.`);
629630
}
630631

631632
if (!response.ok) {
633+
await response.body?.cancel();
632634
throw new Error(`HTTP ${response.status} trying to load well-known OAuth protected resource metadata.`);
633635
}
634636
return OAuthProtectedResourceMetadataSchema.parse(await response.json());
@@ -756,10 +758,12 @@ export async function discoverOAuthMetadata(
756758
});
757759

758760
if (!response || response.status === 404) {
761+
await response?.body?.cancel();
759762
return undefined;
760763
}
761764

762765
if (!response.ok) {
766+
await response.body?.cancel();
763767
throw new Error(`HTTP ${response.status} trying to load well-known OAuth metadata`);
764768
}
765769

@@ -869,6 +873,7 @@ export async function discoverAuthorizationServerMetadata(
869873
}
870874

871875
if (!response.ok) {
876+
await response.body?.cancel();
872877
// Continue looking for any 4xx response code.
873878
if (response.status >= 400 && response.status < 500) {
874879
continue; // Try next URL

‎src/client/sse.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,8 @@ export class SSEClientTransport implements Transport {
253253

254254
const response = await (this._fetch ?? fetch)(this._endpoint, init);
255255
if (!response.ok) {
256+
const text = await response.text().catch(() => null);
257+
256258
if (response.status === 401 && this._authProvider) {
257259
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
258260
this._resourceMetadataUrl = resourceMetadataUrl;
@@ -272,7 +274,6 @@ export class SSEClientTransport implements Transport {
272274
return this.send(message);
273275
}
274276

275-
const text = await response.text().catch(() => null);
276277
throw new Error(`Error POSTing to endpoint (HTTP ${response.status}): ${text}`);
277278
}
278279
} catch (error) {

‎src/client/streamableHttp.test.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -582,11 +582,13 @@ describe('StreamableHTTPClientTransport', () => {
582582
ok: false,
583583
status: 401,
584584
statusText: 'Unauthorized',
585-
headers: new Headers()
585+
headers: new Headers(),
586+
text: async () => Promise.reject('dont read my body')
586587
})
587588
.mockResolvedValue({
588589
ok: false,
589-
status: 404
590+
status: 404,
591+
text: async () => Promise.reject('dont read my body')
590592
});
591593

592594
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
@@ -883,7 +885,8 @@ describe('StreamableHTTPClientTransport', () => {
883885
ok: false,
884886
status: 401,
885887
statusText: 'Unauthorized',
886-
headers: new Headers()
888+
headers: new Headers(),
889+
text: async () => Promise.reject('dont read my body')
887890
};
888891
(global.fetch as Mock)
889892
// Initial connection
@@ -936,7 +939,8 @@ describe('StreamableHTTPClientTransport', () => {
936939
ok: false,
937940
status: 401,
938941
statusText: 'Unauthorized',
939-
headers: new Headers()
942+
headers: new Headers(),
943+
text: async () => Promise.reject('dont read my body')
940944
};
941945
(global.fetch as Mock)
942946
// Initial connection
@@ -962,7 +966,8 @@ describe('StreamableHTTPClientTransport', () => {
962966
// Fallback should fail to complete the flow
963967
.mockResolvedValue({
964968
ok: false,
965-
status: 404
969+
status: 404,
970+
text: async () => Promise.reject('dont read my body')
966971
});
967972

968973
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
@@ -987,7 +992,8 @@ describe('StreamableHTTPClientTransport', () => {
987992
ok: false,
988993
status: 401,
989994
statusText: 'Unauthorized',
990-
headers: new Headers()
995+
headers: new Headers(),
996+
text: async () => Promise.reject('dont read my body')
991997
};
992998
(global.fetch as Mock)
993999
// Initial connection
@@ -1013,7 +1019,8 @@ describe('StreamableHTTPClientTransport', () => {
10131019
// Fallback should fail to complete the flow
10141020
.mockResolvedValue({
10151021
ok: false,
1016-
status: 404
1022+
status: 404,
1023+
text: async () => Promise.reject('dont read my body')
10171024
});
10181025

10191026
await expect(transport.send(message)).rejects.toThrow(UnauthorizedError);
@@ -1026,7 +1033,8 @@ describe('StreamableHTTPClientTransport', () => {
10261033
ok: false,
10271034
status: 401,
10281035
statusText: 'Unauthorized',
1029-
headers: new Headers()
1036+
headers: new Headers(),
1037+
text: async () => Promise.reject('dont read my body')
10301038
};
10311039

10321040
// Create custom fetch
@@ -1328,7 +1336,8 @@ describe('StreamableHTTPClientTransport', () => {
13281336
ok: false,
13291337
status: 401,
13301338
statusText: 'Unauthorized',
1331-
headers: new Headers()
1339+
headers: new Headers(),
1340+
text: async () => Promise.reject('dont read my body')
13321341
};
13331342

13341343
(global.fetch as Mock)

‎src/client/streamableHttp.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ export class StreamableHTTPClientTransport implements Transport {
223223
});
224224

225225
if (!response.ok) {
226+
await response.body?.cancel();
227+
226228
if (response.status === 401 && this._authProvider) {
227229
// Need to authenticate
228230
return await this._authThenStart();
@@ -463,6 +465,8 @@ export class StreamableHTTPClientTransport implements Transport {
463465
}
464466

465467
if (!response.ok) {
468+
const text = await response.text().catch(() => null);
469+
466470
if (response.status === 401 && this._authProvider) {
467471
// Prevent infinite recursion when server returns 401 after successful auth
468472
if (this._hasCompletedAuthFlow) {
@@ -525,7 +529,6 @@ export class StreamableHTTPClientTransport implements Transport {
525529
}
526530
}
527531

528-
const text = await response.text().catch(() => null);
529532
throw new Error(`Error POSTing to endpoint (HTTP ${response.status}): ${text}`);
530533
}
531534

@@ -535,6 +538,7 @@ export class StreamableHTTPClientTransport implements Transport {
535538

536539
// If the response is 202 Accepted, there's no body to process
537540
if (response.status === 202) {
541+
await response.body?.cancel();
538542
// if the accepted notification is initialized, we start the SSE stream
539543
// if it's supported by the server
540544
if (isInitializedNotification(message)) {
@@ -609,6 +613,7 @@ export class StreamableHTTPClientTransport implements Transport {
609613
};
610614

611615
const response = await (this._fetch ?? fetch)(this._url, init);
616+
await response.body?.cancel();
612617

613618
// We specifically handle 405 as a valid response according to the spec,
614619
// meaning the server does not support explicit session termination

‎src/examples/server/elicitationUrlExample.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,8 @@ const tokenVerifier = {
253253
});
254254

255255
if (!response.ok) {
256-
throw new Error(`Invalid or expired token: ${await response.text()}`);
256+
const text = await response.text().catch(() => null);
257+
throw new Error(`Invalid or expired token: ${text}`);
257258
}
258259

259260
const data = await response.json();

‎src/examples/server/simpleStreamableHttp.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,8 @@ if (useOAuth) {
485485
});
486486

487487
if (!response.ok) {
488-
throw new Error(`Invalid or expired token: ${await response.text()}`);
488+
const text = await response.text().catch(() => null);
489+
throw new Error(`Invalid or expired token: ${text}`);
489490
}
490491

491492
const data = await response.json();

‎src/server/auth/providers/proxyProvider.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
8484
},
8585
body: params.toString()
8686
});
87+
await response.body?.cancel();
8788

8889
if (!response.ok) {
8990
throw new ServerError(`Token revocation failed: ${response.status}`);
@@ -107,6 +108,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
107108
});
108109

109110
if (!response.ok) {
111+
await response.body?.cancel();
110112
throw new ServerError(`Client registration failed: ${response.status}`);
111113
}
112114

@@ -181,6 +183,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
181183
});
182184

183185
if (!response.ok) {
186+
await response.body?.cancel();
184187
throw new ServerError(`Token exchange failed: ${response.status}`);
185188
}
186189

@@ -221,6 +224,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
221224
});
222225

223226
if (!response.ok) {
227+
await response.body?.cancel();
224228
throw new ServerError(`Token refresh failed: ${response.status}`);
225229
}
226230

0 commit comments

Comments
 (0)