SignalR Stateful Reconnect API

[BrennanConroy]

Background and Motivation

These are the APIs to support SignalR Stateful Reconnect. The high level is; the low level connection layer needs to allow connections to reconnect, the high level SignalR layer needs to buffer and ack messages.

Proposed API

Server APIs:

namespace Microsoft.AspNetCore.Http.Connections;

public class HttpConnectionDispatcherOptions
{
+   public bool AllowReconnects { get; set; }
}

namespace Microsoft.AspNetCore.SignalR;

public class HubOptions
{
+   public long MessageBufferSize { get; set; } = 100_000;
}

namespace Microsoft.AspNetCore.SignalR;

public class HubConnectionContextOptions
{
+   public long MessageBufferSize { get; set; } = 100_000;
}

Client APIs:

// TS Client
export interface IHttpConnectionOptions {
+   useReconnects?: boolean;
}

// TS Client
export class HubConnectionBuilder {
+   public withMessageBufferSize(messageBufferSize: number): HubConnectionBuilder;
}

namespace Microsoft.AspNetCore.Http.Connections.Client;

public class HttpConnectionOptions
{
+   public bool UseReconnects { get; set; }
}

namespace Microsoft.AspNetCore.SignalR.Client;

public static class HubConnectionBuilderExtensions
{
+   public static IHubConnectionBuilder WithMessageBufferSize(this IHubConnectionBuilder hubConnectionBuilder, long messageBufferSize)
}

Common APIs:

namespace Microsoft.AspNetCore.Http.Connections;

public class NegotiationResponse
{
+   public bool UseReconnects { get; set; }
}

namespace Microsoft.AspNetCore.SignalR.Protocol;

public static class HubProtocolConstants
{
+   public const int AckMessageType = 8;
+   public const int SequenceMessageType = 9;
}

namespace Microsoft.AspNetCore.SignalR.Protocol;

+ public sealed class AckMessage : HubMessage
{
+   public AckMessage(long sequenceId);

+   public long SequenceId { get; set; }
}

+ public sealed class SequenceMessage : HubMessage
{
+   public SequenceMessage(long sequenceId);

+   public long SequenceId { get; set; }
}

namespace Microsoft.AspNetCore.Connections.Abstractions;

#if NET8_0_OR_GREATER
+ [RequiresPreviewFeatures("IReconnectFeature is a preview interface")]
#endif
+ public interface IReconnectFeature
{
+   public Action<PipeWriter> NotifyOnReconnect { get; set; }

+   void DisableReconnect();
}

Usage Examples

99% use case:

// Client
var hubConnection = new HubConnectionBuilder()
    .WithMessageBufferSize(1500)
    .WithUrl("https://example.com", o => o.UseReconnects = true)
    .WithAutomaticReconnects()
    .Build();

// Server
services.AddSignalR(o => o.MessageBufferSize = 1500);

app.MapHub<MyHub>("/default", o => o.AllowReconnects = true);

Other use case:

class MyHub : Hub
{
    void SomeMethod()
    {
        Context.Features.Get<IReconnectFeature>()?.DisableReconnect();
    }
}

Super rare use case:

public class CustomConnectionHandler : ConnectionHandler
{
    private PipeWriter _writer;

    public override async Task OnConnectedAsync(ConnectionContext connection)
    {
        _writer = connection.Application.Output;
        var reconnectFeature = connection.Features.Get<IReconnectFeature>();
        if (reconnectFeature is not null)
        {
            reconnectFeature.NotifyOnReconnect = Reconnected;
        }

        while (true)
        {
            var result await connection.Transport.Input.ReadAsync();

            await _writer.WriteAsync(new byte[]);

            connection.Transport.Input.AdvanceTo(buffer.End);
        }
    }

    private void Reconnected(PipeWriter newWriter)
    {
        _writer.Complete();
        _writer = newWriter;
    }
}

Alternative Designs

AllowAcks and UseAcks are the current API, but that bleeds naming from the SignalR layer (where acks are used) to the connection layer.

Risks

IReconnectFeature is marked experimental because the interaction between the ConnectionHandler and SignalR layer is complex and we aren't sure this is the best way to communicate a reconnect occurred.

MessageBufferSize feels a bit generic. Additionally, we might add a limit on the number of messages (regardless of byte buffer limit) so naming could be very confusing with that limit too.

UseReconnect might want to be a number instead of bool to allow future changes, e.g. support long polling and sse. Also, this is a client API and it's asking to be allowed to use the reconnect feature, maybe the name should reflect that; NegotiateReconnect

[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:

• Is AllowReconnects on by default on the server and/or client?
  • No to both currently. It seems like it could make certain kinds of DoS easier.
• AllowReconnects is maybe a little to broad of a name considering WithAutomaticReconnect on the client works with or without it.
  • AllowStatefulReconnects may be better.
• AllowReconnects could be an int instead of a bool to encode the version.
  • The server should support multiple versions at once if versioning is a thing.
• What's the unit for MessageBufferSize?
  • Bytes.
  • It's not clear that it's bytes especially considering that SignalR used to have a DefaultMessageBufferSize which counted the number of messages that would be stored in the ring buffer and not the bytes.
• Is MessageBufferSize per server? Per client? Per connection?
  • Connection level.
• Why is MessageBufferSize on both HubOptions and HubConnectionContextOptions?
  • That's the pattern for HubOptions. The parameters get copied automatically, so HubConnectionContextOptions.MessageBufferSize doesn't need to be public but it might be nice for testing.
  • Let's make the context one internal like TimeProvider for now.
• StatefulReconnectBufferSize might be a better name MessageBufferSize
• How do we set more options like the number of retry attempts? Are they all going to be HttpConnectionOptions and IHttpConnectionOptions.
• IReconnectFeature -> IStatefulReconnectFeature
• UseReconnects->UseStatefulReconnects
• Consider adding a WithStatefulReconnects(Action<StatefulReconnectOptions>? configure = null) method to the client. It looks like this won't require a new assembly.

We'll take a look at this again after we've made some of the suggested updates.

[BrennanConroy]

Server APIs:

namespace Microsoft.AspNetCore.Http.Connections;

public class HttpConnectionDispatcherOptions
{
+   public bool AllowStatefulReconnects { get; set; }
}

namespace Microsoft.AspNetCore.SignalR;

public class HubOptions
{
+   public long StatefulReconnectBufferSize { get; set; } = 100_000;
}

Client APIs:

// TS Client
export interface IHttpConnectionOptions {
+   useStatefulReconnect?: boolean;
}

// TS Client
export class HubConnectionBuilder {
+   public withStatefulReconnect(options?: StatefulReconnectOptions): HubConnectionBuilder;
}

+ export interface IStatefulReconnectOptions{
+   statefulReconnectBufferSize: number;
}

namespace Microsoft.AspNetCore.Http.Connections.Client;

public class HttpConnectionOptions
{
+   public bool UseStatefulReconnect { get; set; }
}

+ public sealed class HubConnectionOptions
{
+   public long StatefulReconnectBufferSize { get; set; }
}

namespace Microsoft.AspNetCore.SignalR.Client;

public static class HubConnectionBuilderHttpExtensions
{
+   public static IHubConnectionBuilder WithStatefulReconnect(this IHubConnectionBuilder hubConnectionBuilder, StatefulReconnectOptions? statefulReconnectOptions = null);
}

namespace Microsoft.AspNetCore.SignalR.Client;

+ public sealed class StatefulReconnectOptions
{
+   public long StatefulReconnectBufferSize { get; set; }
}

Common APIs:

namespace Microsoft.AspNetCore.Http.Connections;

public class NegotiationResponse
{
+   public bool UseStatefulReconnect { get; set; }
}

namespace Microsoft.AspNetCore.SignalR.Protocol;

public static class HubProtocolConstants
{
+   public const int AckMessageType = 8;
+   public const int SequenceMessageType = 9;
}

namespace Microsoft.AspNetCore.SignalR.Protocol;

+ public sealed class AckMessage : HubMessage
{
+   public AckMessage(long sequenceId);

+   public long SequenceId { get; set; }
}

+ public sealed class SequenceMessage : HubMessage
{
+   public SequenceMessage(long sequenceId);

+   public long SequenceId { get; set; }
}

namespace Microsoft.AspNetCore.Connections.Abstractions;

#if NET8_0_OR_GREATER
+ [RequiresPreviewFeatures("IStatefulReconnectFeature is a preview interface")]
#endif
+ public interface IStatefulReconnectFeature
{
+   public Action<PipeWriter> NotifyOnReconnect { get; set; }

+   void DisableReconnect();
}

Usage Examples

99% use case:

// Client
var hubConnection = new HubConnectionBuilder()
    .WithStatefulReconnect(new StatefulReconnectOptions() { StatefulReconnectBufferSize = 5000 })
    .WithUrl("https://example.com")
    .WithAutomaticReconnects()
    .Build();

// Server
services.AddSignalR(o => o.StatefulReconnectBufferSize = 1500);

app.MapHub<MyHub>("/default", o => o.AllowStatefulReconnects = true);

[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.

[davidfowl]

IStatefulReconnectFeature should be:

public interface IStatefulReconnectFeature
{
    public void OnReconnect(Action<PipeWriter> writer);

    void DisableReconnect();
}

Should it be Func<PipeWriter, Task>?

[davidfowl]

StatefulReconnectOptions is missing in the API proposal.

[halter73]

API Review Notes:

• Are we really okay with statefulReconnectBufferSize without a unit?
  • If it was messages we'd use MessageCount or something.
  • We still think doc comments are sufficient for units.
  • It's consistent with KestrelServerLimits.
• Should we include the word "Max" in statefulReconnectBufferSize since the buffer does shrink for idle connections?
  • Kestrel's MaxResponseBufferSize is similar, but no one seems to think it's that valuable.
• Do we want to expose the other currently-internal HubConnectionOptions?
  • Yes. For consistency.
• In WithStatefulReconnect(this IHubConnectionBuilder hubConnectionBuilder, StatefulReconnectOptions? statefulReconnectOptions = null), should we configure the options with a callback?
  • WithStatefulReconnect(this IHubConnectionBuilder hubConnectionBuilder, Action<StatefulReconnectOptions>? configure = null)
  • We'll leave the TS withStatefulReconnect as taking the options directly then.
• Should we use the options pattern for StatefulReconnectOptions too?
  • These options get copied to "real" options, so this isn't really possible without hacks.
  • Can we just remove StatefulReconnectOptions in the C# client for now and make people call .Configure<HubConnectionOptions>(o => ...) to reconfigure the buffer limits.
• Is statefulReconnectBufferSize redundant considering it's part of the options passed to withStatefulReconnect?
• Can we increase the statefulReconnectBufferSize on the client to something larger than (we think) anyone will need and make it non-configurable?
  • 10 MB?
  • Could we do the same for the C# client?
• Is there an event on the clients for when they are trying a stateful reconnect?
  • Not yet. But we like OnConnectionSlow 😝
  • If we allow a small statefulReconnectBufferSize, you could use backpressure as a substitute for an OnConnectionSlow-like API
• Should HubMessage's have public setters on the properties?
  • We already do this for StreamItem, so this isn't new.
• NotifyOnReconnect property on the feature should be an OnReconnected method that takes a Task-returning func. This is a preview feature, so it might need redesign to not take the new output PipeWriter.
• Let's make the TS IHttpConnectionOptions internal.

API Approved!

Server APIs:

namespace Microsoft.AspNetCore.Http.Connections;

public class HttpConnectionDispatcherOptions
{
+   public bool AllowStatefulReconnects { get; set; }
}

namespace Microsoft.AspNetCore.SignalR;

public class HubOptions
{
+   public long StatefulReconnectBufferSize { get; set; } = 100_000;
}

// TS Client
export class HubConnectionBuilder {
+   public withStatefulReconnect(options?: StatefulReconnectOptions): HubConnectionBuilder;
}

+ export interface IStatefulReconnectOptions {
+   bufferSize: number;
+ }

namespace Microsoft.AspNetCore.Http.Connections.Client;

public class HttpConnectionOptions
{
+   public bool UseStatefulReconnect { get; set; }
}

+ public sealed class HubConnectionOptions
{
+   public long StatefulReconnectBufferSize { get; set; } 
+   public TimeSpan ServerTimeout { get; set; }
+   public TimeSpan KeepAliveInterval { get; set; }
}

namespace Microsoft.AspNetCore.SignalR.Client;

public static class HubConnectionBuilderHttpExtensions
{
+   public static IHubConnectionBuilder WithStatefulReconnect(this IHubConnectionBuilder hubConnectionBuilder);
}

Common APIs:

namespace Microsoft.AspNetCore.Http.Connections;

public class NegotiationResponse
{
+   public bool UseStatefulReconnect { get; set; }
}

namespace Microsoft.AspNetCore.SignalR.Protocol;

public static class HubProtocolConstants
{
+   public const int AckMessageType = 8;
+   public const int SequenceMessageType = 9;
}

namespace Microsoft.AspNetCore.SignalR.Protocol;

+ public sealed class AckMessage : HubMessage
{
+   public AckMessage(long sequenceId);

+   public long SequenceId { get; set; }
}

+ public sealed class SequenceMessage : HubMessage
{
+   public SequenceMessage(long sequenceId);

+   public long SequenceId { get; set; }
}

namespace Microsoft.AspNetCore.Connections.Abstractions;

#if NET8_0_OR_GREATER
+ [RequiresPreviewFeatures("IStatefulReconnectFeature is a preview interface")]
#endif
+ public interface IStatefulReconnectFeature
{
+   public void OnReconnected(Func<PipeWriter, Task> notifyOnReconnect);
+   void DisableReconnect();
}