Skip to content

Commit 0824dfb

Browse files
fix: address round-5 review comments on 401 edge cases
Three fixes from claude[bot] review: 1. _startOrAuthSse 405 return doesn't reset _authRetryInFlight — 401 → onUnauthorized → retry → 405 would leave the flag set forever, disabling onUnauthorized for subsequent send() 401s. Added reset before the 405 return. 2. SSE onerror handler — when onUnauthorized rejects in the connect path, the error went through .then(resolve, reject) without calling this.onerror. Every other error path in both transports calls both. Added this.onerror?.() before reject. 3. 401 with no authProvider drained body twice — the 401 block ran unconditionally, drained at line 232/285/521, fell through (no authProvider = no throw), then the generic error handler drained again (empty) and produced 'HTTP 401: null'. Gated the entire 401 block on this._authProvider (matching pre-PR structure) so no-auth 401s hit the generic error directly with intact body text.
1 parent 3aa0cd6 commit 0824dfb

2 files changed

Lines changed: 14 additions & 16 deletions

File tree

packages/client/src/client/sse.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,10 @@ export class SSEClientTransport implements Transport {
154154
this._authProvider
155155
.onUnauthorized({ response, serverUrl: this._url, fetchFn: this._fetchWithInit })
156156
.then(() => this._startOrAuth())
157-
.then(resolve, reject)
157+
.then(resolve, error => {
158+
this.onerror?.(error);
159+
reject(error);
160+
})
158161
.finally(() => {
159162
this._authRetryInFlight = false;
160163
});
@@ -261,14 +264,14 @@ export class SSEClientTransport implements Transport {
261264

262265
const response = await (this._fetch ?? fetch)(this._endpoint, init);
263266
if (!response.ok) {
264-
if (response.status === 401) {
267+
if (response.status === 401 && this._authProvider) {
265268
if (response.headers.has('www-authenticate')) {
266269
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
267270
this._resourceMetadataUrl = resourceMetadataUrl;
268271
this._scope = scope;
269272
}
270273

271-
if (this._authProvider?.onUnauthorized && !this._authRetryInFlight) {
274+
if (this._authProvider.onUnauthorized && !this._authRetryInFlight) {
272275
this._authRetryInFlight = true;
273276
await this._authProvider.onUnauthorized({
274277
response,
@@ -285,9 +288,7 @@ export class SSEClientTransport implements Transport {
285288
status: 401
286289
});
287290
}
288-
if (this._authProvider) {
289-
throw new UnauthorizedError();
290-
}
291+
throw new UnauthorizedError();
291292
}
292293

293294
const text = await response.text?.().catch(() => null);

packages/client/src/client/streamableHttp.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,14 @@ export class StreamableHTTPClientTransport implements Transport {
211211
});
212212

213213
if (!response.ok) {
214-
if (response.status === 401) {
214+
if (response.status === 401 && this._authProvider) {
215215
if (response.headers.has('www-authenticate')) {
216216
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
217217
this._resourceMetadataUrl = resourceMetadataUrl;
218218
this._scope = scope;
219219
}
220220

221-
if (this._authProvider?.onUnauthorized && !this._authRetryInFlight) {
221+
if (this._authProvider.onUnauthorized && !this._authRetryInFlight) {
222222
this._authRetryInFlight = true;
223223
await this._authProvider.onUnauthorized({
224224
response,
@@ -235,16 +235,15 @@ export class StreamableHTTPClientTransport implements Transport {
235235
status: 401
236236
});
237237
}
238-
if (this._authProvider) {
239-
throw new UnauthorizedError();
240-
}
238+
throw new UnauthorizedError();
241239
}
242240

243241
await response.text?.().catch(() => {});
244242

245243
// 405 indicates that the server does not offer an SSE stream at GET endpoint
246244
// This is an expected case that should not trigger an error
247245
if (response.status === 405) {
246+
this._authRetryInFlight = false;
248247
return;
249248
}
250249

@@ -500,15 +499,15 @@ export class StreamableHTTPClientTransport implements Transport {
500499
}
501500

502501
if (!response.ok) {
503-
if (response.status === 401) {
502+
if (response.status === 401 && this._authProvider) {
504503
// Store WWW-Authenticate params for interactive finishAuth() path
505504
if (response.headers.has('www-authenticate')) {
506505
const { resourceMetadataUrl, scope } = extractWWWAuthenticateParams(response);
507506
this._resourceMetadataUrl = resourceMetadataUrl;
508507
this._scope = scope;
509508
}
510509

511-
if (this._authProvider?.onUnauthorized && !this._authRetryInFlight) {
510+
if (this._authProvider.onUnauthorized && !this._authRetryInFlight) {
512511
this._authRetryInFlight = true;
513512
await this._authProvider.onUnauthorized({
514513
response,
@@ -525,9 +524,7 @@ export class StreamableHTTPClientTransport implements Transport {
525524
status: 401
526525
});
527526
}
528-
if (this._authProvider) {
529-
throw new UnauthorizedError();
530-
}
527+
throw new UnauthorizedError();
531528
}
532529

533530
const text = await response.text?.().catch(() => null);

0 commit comments

Comments
 (0)