API proposal: enable configuring connection timeouts in HubConnectionBuilder

[surayya-MS]

Background and Motivation

Related to #18840

When debugging a server-side blazor app customers often see this error pop up in the client (browser) while stepping through code: Error: Connection disconnected with error 'Error: Server timeout elapsed without receiving a message from the server.'. This can be disruptive, requiring a refresh of the browser to recover after continuing the debug session.

The workaround for this is to set serverTimeoutInMilliseconds for HubConnection in Blazor.start.

Blazor.start({
    configureSignalR: function (builder) {
        let c = builder.build();
        c.serverTimeoutInMilliseconds = 60000;
        c.keepAliveIntervalInMilliseconds = 30000;
        builder.build = () => {
            return c;
        };
    };
})

Instead of overriding build method of HubConnectionBuilder with set serverTimeoutInMilliseconds it would be nice to set this things directly in the builder:

Blazor.start({
    configureSignalR: function (builder) {
        builder.serverTimeoutInMilliseconds = 60000;
        builder.keepAliveIntervalInMilliseconds = 30000;
    };
});

Proposed API

TypeScript Client

Enable configuring the connection timeouts in HubConnectionBuilder like serverTimeoutInMilliseconds and keepAliveIntervalInMilliseconds .

class HubConnectionBuilder {
+    public serverTimeoutInMilliseconds?: number;
+
+    public keepAliveIntervalInMilliseconds?: number;

    public reconnectPolicy?: IRetryPolicy;

    public configureLogging(logLevel: LogLevel): HubConnectionBuilder;

    public withUrl(url: string): HubConnectionBuilder;

    public withAutomaticReconnect(): HubConnectionBuilder;
}

C# Client

public class HubConnectionBuilder : IHubConnectionBuilder
{
    public IServiceCollection Services { get; }

+    public TimeSpan? ServerTimeout { get; set; }

+    public TimeSpan? KeepAliveInterval { get; set; }

    public HubConnectionBuilder();

    public HubConnection Build();
}

public static class HubConnectionBuilderExtensions
{
    public static IHubConnectionBuilder ConfigureLogging(this IHubConnectionBuilder hubConnectionBuilder, Action<ILoggingBuilder> configureLogging);

    public static IHubConnectionBuilder WithAutomaticReconnect(this IHubConnectionBuilder hubConnectionBuilder, TimeSpan[] reconnectDelays);
}

Usage Examples

Customers will use it to configure the connection timeout in server-side blazor app like so:

Blazor.start({
    configureSignalR: function (builder) {
        builder.serverTimeoutInMilliseconds = 60000;
        builder.keepAliveIntervalInMilliseconds = 30000;
    };
});

Alternative Designs

As an alternative we can add a delegate to Blazor.start to configure HubConnection

Blazor.start({
            configureConnection: function (connection) {
                connection.serverTimeoutInMilliseconds = 60000;
                connection.keepAliveIntervalInMilliseconds = 30000;
            },
        });

Risks

I'm don't think there are any

[surayya-MS]

@BrennanConroy , any updates on this API proposal? Do you think this is worth implementing or should we implement the alternative design (https://github.com/dotnet/aspnetcore/pull/44623)?

[BrennanConroy]

We need to put what the API will look like in the issue. Example: https://github.com/dotnet/aspnetcore/issues/43355#issue-1342386431

Also, we probably want to do this across all the clients, not just the Typescript one.

[surayya-MS]

Included what API will look like for TypeScript and C# clients. Also opened a PR for it. If the proposed design looks good I will proceed with Java client.

[BrennanConroy]

Thanks, simplified it because we don't need to see the implementation (and all the doc comments etc.) during API review.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• This is particularly important to blazor which doesn't expose the connection. This could allow unwanted interference with the blazor app
• Do we need the C# client changes? No, but it's more consistent.
• Should we use a new option type? Or methods to set these properties? Let's use methods. It does not allow reading the setting back of the builder, but we don't think this is important. It can still be read off of the HubConnection.
• Is there more config we want to add the the builders that are currently on the HubConnection type? HandshakeTimeout? We can add HandshakeTimeout later.

TS

export class HubConnectionBuilder
{
+    public withServerTimeout(milliseconds: number): HubConnectionBuilder;
+    public withKeepAliveInterval(milliseconds: number): HubConnectionBuilder;
}

Example Usage:

Blazor.start({
    configureSignalR: function (builder) {
        builder.withServerTimeout(60000).withKeepAliveInterval(3000);
    };
});

C#

namespace Microsoft.AspNetCore.SignalR.Client;

public static class HubConnectionBuilderExtensions
{
+   public static IHubConnectionBuilder WithServerTimeout(this IHubConnectionBuilder hubConnectionBuilder, TimeSpan timeout);
+   public static IHubConnectionBuilder WithKeepAliveInterval(this IHubConnectionBuilder hubConnectionBuilder, TimeSpan interval);

Java

We also approve the equivalent change to Java's HttpHubConnectionBuilder.

API Approved!

[halter73]

We had an internal conversation about moving the C# client's HubConnectionBuilderHttpExtensions methods to HubConnectionBuilderExtensions. @surayya-MS pointed out it would be more consistent this way.

In HubConnectionBuilderHttpExtensions there are only methods to set Url which is done by configuring HttpConnectionOptions. I needed to save ServerTimeout and KeepAliveInterval somewhere so then l could use them to set same properties in HubConnection. At first I considered adding those to HttpConnectionOptions but HttpConnectionOptions is used not only for HubConnection. That's why I created internal class HubConnectionOptions. And since there is no relation to Http it made more sense to add the new methods to HubConnectionBuilderExtensions instead of HubConnectionBuilderHttpExtensions.

The API reviewers agreed that it makes sense to move the methods. The updated approved API for the C# client is now:

C#

namespace Microsoft.AspNetCore.SignalR.Client;

public static class HubConnectionBuilderExtensions
{
+   public static IHubConnectionBuilder WithServerTimeout(this IHubConnectionBuilder hubConnectionBuilder, TimeSpan timeout);
+   public static IHubConnectionBuilder WithKeepAliveInterval(this IHubConnectionBuilder hubConnectionBuilder, TimeSpan interval);

I've updated the original API approval comment to reflect this.