Allow IReconnectPolicy.nextRetryDelayInMilliseconds to return a promise in the SignalR TS client

[halter73]

Epic #8797

See https://github.com/aspnet/AspNetCore/pull/8566/files#r269656331

The following is the current signature for IReconnectPolicy.nextRetryDelayInMilliseconds

/** An abstraction that controls when the client attempts to reconnect and how many times it does so. */
export interface IReconnectPolicy {
    /** Called after the transport loses the connection.
     *
     * @param {number} previousRetryCount The number of failed reconnect attempts so far.
     *
     * @param {number} elapsedMilliseconds The amount of time in milliseconds spent reconnecting so far.
     *
     * @returns {number | null} The amount of time in milliseconds to wait before the next reconnect attempt. `null` tells the client to stop retrying and close.
     */
    nextRetryDelayInMilliseconds(previousRetryCount: number, elapsedMilliseconds: number): number | null;
}

The proposal is to change it to:

    nextRetryDelayInMilliseconds(previousRetryCount: number, elapsedMilliseconds: number): number | Promise<void> | null

If a promise is returned instead of a number, we'd await the promise instead of the set number of milliseconds.

[davidfowl]

Shouldn't this part of the the reconnect epic cc @anurse

[halter73]

The only reason I didn't put this and #9016 in the epic is because there's a very realistic possibility we decide it's not worthwhile to address either these issues. I filed these issues to get them out of the auto-reconnect epic so they wouldn't be considered as part of #8709.

Based on my discussions so far, I don't think requiring that nextRetryDelayInMilliseconds returns a number instead of a promise limits any real use cases. But a good use case could convince me otherwise.

A ReconnectPolicyBuilder as suggested in #9016 would be nice to have to fill in the gap between the two current withAutomaticReconnect() overloads and setting a custom IReconnectPolicy. I'm just not sure if it's nice enough to devote time to for 3.0.

I plan to bring this up in the next triage. If we decide not to do this, we can just close the issue. I put #9016 in the epic for now for consistency.

[analogrelay]

It's still OK to put these in the epic if we're not sure about them, but agreed, we should talk in triage :)

[analogrelay]

Triage decision: JS allows us to add this safely in any version, so we'll wait for customer feedback here.

[bigswede74]

Was this feature ever implemented? The documentation states that the JS client has the below feature, is this available in TS client?

const connection = new signalR.HubConnectionBuilder()
    .withUrl("/chatHub")
    .withAutomaticReconnect()
    .build();

[BrennanConroy]

Yes, it's available. The JS client is built from the TS client.

[bigswede74]

@BrennanConroy Am I missing an import? I get TS compile errors when adding the line to the HubConnectionBuilder.

.withAutomaticReconnect()

[BrennanConroy]

Which version of the npm package are you using?

[bigswede74]

@BrennanConroy I'm using below.

"@aspnet/signalr": "^1.1.4",

[BrennanConroy]

This feature is only available in >=3.0.0

[bigswede74]

@BrennanConroy I just ran the below command. why is it not garbing the most recent release package?

npm install @aspnet/signalr