fix(SignalR/TS): 401 retry + no-token path cleans Authorization and resends; shrink bundle

fix(SignalR): ensure access token factory is always called and improve error handling in tests

Author: daniloneto

src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts

@@ -1,28 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-import { HeaderNames } from "./HeaderNames";
+// Removed HeaderNames import to reduce bundle size; using literal key.
 import { HttpClient, HttpRequest, HttpResponse } from "./HttpClient";
 
-// Internal helpers for Error type guarding and status normalization
-function isError(u: unknown): u is Error {
-    return u instanceof Error;
-}
-function getStatus(u: unknown): number | undefined {
-    if (typeof u !== "object" || u === null) { return undefined; }
-    const rec = u as Record<string, unknown>;
-    const raw = rec["statusCode"] ?? rec["status"];
-    if (typeof raw === "number") { return raw; }
-    if (typeof raw === "string") {
-        const n = parseInt(raw, 10);
-        return Number.isNaN(n) ? undefined : n;
-    }
-    return undefined;
-}
-
 /** @private */
 export class AccessTokenHttpClient extends HttpClient {
-    private _innerClient: HttpClient;
+    private readonly _innerClient: HttpClient;
     _accessToken: string | undefined;
     _accessTokenFactory: (() => string | Promise<string>) | undefined;
 
@@ -34,62 +18,38 @@ export class AccessTokenHttpClient extends HttpClient {
     }
 
     public async send(request: HttpRequest): Promise<HttpResponse> {
-        let allowRetry = true;
-        if (this._accessTokenFactory && (!this._accessToken || (request.url && request.url.indexOf("/negotiate?") > 0))) {
-            // don't retry if the request is a negotiate or if we just got a potentially new token from the access token factory
-            allowRetry = false;
-            this._accessToken = await this._accessTokenFactory();
-        }
+        const needsToken = !!(this._accessTokenFactory && (!this._accessToken || (request.url && request.url.indexOf('/negotiate?') > 0)));
+        const retry = !needsToken;
+        if (needsToken) this._accessToken = await this._accessTokenFactory!();
         this._setAuthorizationHeader(request);
-
         try {
-            const response = await this._innerClient.send(request);
-
-            if (allowRetry && this._accessTokenFactory && response.statusCode === 401) {
-                return await this._refreshTokenAndRetry(request, response);
-            }
-            return response;
+            const r = await this._innerClient.send(request);
+            return (retry && this._accessTokenFactory && r.statusCode === 401) ? this._refreshTokenAndRetry(request, r) : r;
         } catch (err: unknown) {
-            if (!allowRetry || !this._accessTokenFactory) {
-                throw err;
-            }
-            if (!isError(err)) {
-                throw err;
-            }
-            const status = getStatus(err);
-            if (status === 401) {
-                return await this._refreshTokenAndRetry(request, err);
-            }
+            if (!retry || !this._accessTokenFactory) throw err;
+            const e = err as any, s = +(e.statusCode ?? e.status);
+            if (s === 401) return this._refreshTokenAndRetry(request, e);
             throw err;
         }
     }
 
-    private async _refreshTokenAndRetry(request: HttpRequest, original: HttpResponse | Error): Promise<HttpResponse> {
-        const newToken = await this._accessTokenFactory();
-        if (!newToken) {
-            if (original instanceof HttpResponse) {
-                return original;
-            }
-            throw original;
+    private async _refreshTokenAndRetry(request: HttpRequest, o: HttpResponse | Error): Promise<HttpResponse> {
+        const t = await this._accessTokenFactory!();
+        if (!t) {
+            this._accessToken = undefined;
+            if (request.headers) delete (request.headers as any).Authorization;
+            if (request.abortSignal) return this._innerClient.send(request);
+            if (o instanceof HttpResponse) return o;
+            return Promise.reject(o);
         }
-        this._accessToken = newToken;
+        this._accessToken = t;
         this._setAuthorizationHeader(request);
-        return await this._innerClient.send(request);
+        return this._innerClient.send(request);
     }
 
     private _setAuthorizationHeader(request: HttpRequest) {
-        if (!request.headers) {
-            request.headers = {};
-        }
-        if (this._accessToken) {
-            request.headers[HeaderNames.Authorization] = `Bearer ${this._accessToken}`
-        }
-        // don't remove the header if there isn't an access token factory, the user manually added the header in this case
-        else if (this._accessTokenFactory) {
-            if (request.headers[HeaderNames.Authorization]) {
-                delete request.headers[HeaderNames.Authorization];
-            }
-        }
+        const h = request.headers || (request.headers = {});
+        if (this._accessToken) h.Authorization = 'Bearer ' + this._accessToken; else if (this._accessTokenFactory) delete h.Authorization;
     }
 
     public getCookieString(url: string): string {

src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts

@@ -72,8 +72,7 @@ describe("AccessTokenHttpClient", () => {
 
             await client.get("http://example.com/prime");
             try {
-                await client.get("http://example.com/resource");
-                expect.fail("expected to throw");
+                await expect(client.get("http://example.com/resource")).rejects.toThrow(HttpError);
             } catch (e: any) {
                 expect(e).toBeInstanceOf(HttpError);
                 expect(e.statusCode ?? e.status).toBe(status);