Skip to content

fix(client): replace body.cancel() with text() to prevent hanging #1320

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.

Sign up for GitHub

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

Open
grimmerk wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −13
Open

fix(client): replace body.cancel() with text() to prevent hanging #1320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/client/src/client/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,12 @@ export async function discoverOAuthProtectedResourceMetadata(
});

if (!response || response.status === 404) {
await response?.body?.cancel();
await response?.text?.().catch(() => {});
throw new Error(`Resource server does not implement OAuth 2.0 Protected Resource Metadata.`);
}

if (!response.ok) {
await response.body?.cancel();
await response.text?.().catch(() => {});
throw new Error(`HTTP ${response.status} trying to load well-known OAuth protected resource metadata.`);
}
return OAuthProtectedResourceMetadataSchema.parse(await response.json());
Expand Down Expand Up @@ -803,12 +803,12 @@ export async function discoverOAuthMetadata(
});

if (!response || response.status === 404) {
await response?.body?.cancel();
await response?.text?.().catch(() => {});
return undefined;
}

if (!response.ok) {
await response.body?.cancel();
await response.text?.().catch(() => {});
throw new Error(`HTTP ${response.status} trying to load well-known OAuth metadata`);
}

Expand Down Expand Up @@ -918,7 +918,7 @@ export async function discoverAuthorizationServerMetadata(
}

if (!response.ok) {
await response.body?.cancel();
await response.text?.().catch(() => {});
// Continue looking for any 4xx response code.
if (response.status >= 400 && response.status < 500) {
continue; // Try next URL
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/client/sse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export class SSEClientTransport implements Transport {

const response = await (this._fetch ?? fetch)(this._endpoint, init);
if (!response.ok) {
const text = await response.text().catch(() => null);
const text = await response.text?.().catch(() => null);

if (response.status === 401 && this._authProvider) {
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
Expand All @@ -285,7 +285,7 @@ export class SSEClientTransport implements Transport {
}

// Release connection - POST responses don't have content we need
await response.body?.cancel();
await response.text?.().catch(() => {});
} catch (error) {
this.onerror?.(error as Error);
throw error;
Expand Down
12 changes: 6 additions & 6 deletions packages/client/src/client/streamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export class StreamableHTTPClientTransport implements Transport {
});

if (!response.ok) {
await response.body?.cancel();
await response.text?.().catch(() => {});

if (response.status === 401 && this._authProvider) {
// Need to authenticate
Expand Down Expand Up @@ -495,7 +495,7 @@ export class StreamableHTTPClientTransport implements Transport {
}

if (!response.ok) {
const text = await response.text().catch(() => null);
const text = await response.text?.().catch(() => null);

if (response.status === 401 && this._authProvider) {
// Prevent infinite recursion when server returns 401 after successful auth
Expand Down Expand Up @@ -568,7 +568,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();
await response.text?.().catch(() => {});
// if the accepted notification is initialized, we start the SSE stream
// if it's supported by the server
if (isInitializedNotification(message)) {
Expand Down Expand Up @@ -603,12 +603,12 @@ export class StreamableHTTPClientTransport implements Transport {
this.onmessage?.(msg);
}
} else {
await response.body?.cancel();
await response.text?.().catch(() => {});
throw new StreamableHTTPError(-1, `Unexpected content type: ${contentType}`);
}
} else {
// No requests in message but got 200 OK - still need to release connection
await response.body?.cancel();
await response.text?.().catch(() => {});
}
} catch (error) {
this.onerror?.(error as Error);
Expand Down Expand Up @@ -647,7 +647,7 @@ export class StreamableHTTPClientTransport implements Transport {
};

const response = await (this._fetch ?? fetch)(this._url, init);
await response.body?.cancel();
await response.text?.().catch(() => {});

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