-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Description
Is there an existing issue for this?
- I have searched the existing issues
Describe the bug
In the Browser/TypeScript SignalR client, the Server-Sent Event (SSE) keep alive ping fails to renew the access token (it fails to perform the existing retry logic and call the access token factory again) if a 401 Unauthorised happens on the ping.
This doesn't just affect the SSE ping... Similarly, when Long Polling connections are being reissued after they timeout, this same issue causes the Long Polling to unnecessarily go through a longer lifecycle where is 'completes' / sets this._running to false and closes by calling this.raiseOnClose() - instead of a shorter lifecycle that keeps the _poll function looping in a running state.
When an SSE keep alive ping happens (for example), AccessTokenHttpClient calls this._innerClient
to send a request.
const response = await this._innerClient.send(request); |
Then FetchHttpClient throws an HttpError if the response is not Ok.
throw new HttpError(errorMessage || response.statusText, response.status); |
And there be the problem, because that HttpError is not caught anywhere, so the existing retry logic in AccessTokenHttpClient is prevented from inspecting the response.statusCode for a 401 Unauthorized and performing a retry to get a new access token.
if (allowRetry && response.statusCode === 401 && this._accessTokenFactory) { |
const response = await this._innerClient.send(request); // FetchHttpClient THROWS IN HERE
// SO THIS NEVER EXECUTES:
if (allowRetry && response.statusCode === 401 && this._accessTokenFactory) {
this._accessToken = await this._accessTokenFactory();
this._setAuthorizationHeader(request);
return await this._innerClient.send(request);
}
Given that it is existing retry logic that is not working as intended, I think this is a bug.
Expected Behavior
The existing retry code should execute as apparently intended and call the access token factory.
Steps To Reproduce
- Set up a hub connection (using the TypeScript client in a browser which has fetch available) with the option
transport: HttpTransportType.ServerSentEvents,
- Have a bearer access token with a lifetime shorter than the keep alive ping (or ensure that the ping will return a 401 Unauthorized)
- Let it run and wait for the keep alive ping to happen
- When the ping happens and a 401 is returned, the retry logic in AccessTokenHttpClient.ts does not execute because an HttpError is thrown
if (allowRetry && response.statusCode === 401 && this._accessTokenFactory) {
Exceptions (if any)
HttpError thrown by FetchHttpClient
throw new HttpError(errorMessage || response.statusText, response.status); |
.NET Version
8.0.302
Anything else?
"@microsoft/signalr": "^8.0.0",
I see #5297 is somewhat related - not sure if that will change the logic or the intended approach.