Repository navigation
feat(ConnectionSnapshot): add ConnectionSnapshot response model - #1255
Merged
Merged
Conversation
Reviewer's GuideAdds a new SignalR client extension for WASM that shares a lifecycle-managed HubConnection across generic message dispatch and connection-monitoring services, with automatic reconnect, subscription restoration, heartbeat updates, snapshot polling, and DI registration. Sequence diagram for SignalR dispatch and subscription lifecyclesequenceDiagram
participant App
participant Dispatch as DefaultDispatchService
participant Connection as SignalRConnection
participant Hub as SignalR Hub
participant Subscriber as Local callback
App->>Dispatch: SubscribeAsync(callback)
Dispatch->>Connection: SubscribeAsync(entryType, ReceiveAsync)
Connection->>Hub: Subscribe(entryType)
Hub-->>Connection: subscription result
Connection-->>Dispatch: subscription ready
App->>Dispatch: DispatchAsync(entry)
Dispatch->>Connection: DispatchAsync(entryType, entry)
Connection->>Hub: Dispatch(entryType, entry)
Hub-->>Connection: Receive(entryType, entry)
Connection->>Dispatch: ReceiveAsync(json)
Dispatch->>Subscriber: callback(entry)
App->>Dispatch: UnSubscribeAsync(callback)
Dispatch->>Connection: UnsubscribeAsync(entryType)
Connection->>Hub: Unsubscribe(entryType)
Sequence diagram for connection heartbeat and snapshot pollingsequenceDiagram
participant App
participant Monitor as DefaultConnectionService
participant Connection as SignalRConnection
participant Hub as SignalR Hub
App->>Monitor: AddOrUpdateAsync(client)
Monitor->>Connection: InvokeAsync(Heartbeat, client)
Connection->>Hub: Heartbeat(client)
Hub-->>Connection: success or disabled
loop Every 3 seconds
Monitor->>Connection: InvokeAsync(GetConnectionSnapshot, null)
Connection->>Hub: GetConnectionSnapshot()
Hub-->>Connection: ConnectionSnapshot
Connection-->>Monitor: snapshot
end
App->>Monitor: DisposeAsync()
Monitor->>Connection: ReleaseAsync()
Connection->>Connection: StopAsync when unused
State diagram for shared SignalR connection lifecyclestateDiagram-v2
[*] --> Disconnected
Disconnected --> Connected: EnsureConnectedAsync / StartAsync
Connected --> Reconnecting: SignalR transport loss
Reconnecting --> Connected: RestoreSubscriptionsAsync
Connected --> Disconnected: UnsubscribeAsync or ReleaseAsync
Disconnected --> [*]: DisposeAsync
Reconnecting --> Disconnected: reconnect failure
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![bb-auto[bot]](https://avatars.githubusercontent.com/in/287253?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 8 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/extensions/BootstrapBlazor.SignalR.Client/DefaultConnectionService.cs" line_range="92" />
<code_context>
+ {
+ try
+ {
+ var snapshot = await _connection.InvokeAsync<ConnectionSnapshot?>("GetConnectionSnapshot", null, cancellationToken);
+ if (snapshot == null)
+ {
</code_context>
<issue_to_address>
**Connection snapshots reach every Hub client**
When the host enables connection snapshots, leaves the snapshot authorization policy unset, and allows callers who should not see connection metadata to connect to the Hub, `PollAsync` invokes `GetConnectionSnapshot` and caches the returned client metadata. The server skips its policy check when no snapshot policy is configured, so every caller allowed to connect to that Hub can read the snapshot, including callers without a separate authorization to view it.
Require a dedicated authorization policy before returning snapshots, or ensure the Hub endpoint’s authorization is sufficiently restrictive whenever snapshots are enabled.
</issue_to_address>
### Comment 2
<location path="src/extensions/BootstrapBlazor.SignalR.Client/DefaultDispatchService.cs" line_range="96-102" />
<code_context>
+ }
+ }
+ }
+ catch
+ {
+ // 订阅未就绪时撤销本次本地注册，使后续订阅者仍能走 first 分支重新尝试。
+ // Roll back this registration if subscribing fails so the next subscriber can retry through the first branch.
+ lock (_locker)
+ {
+ _callbacks.Remove(subscription);
+ }
+ throw;
</code_context>
<issue_to_address>
**Server group subscriptions leak**
When the subscribe request is canceled or fails after the server adds the connection to the group, `SubscribeAsync` removes the local callback but leaves `_requiresUnsubscribe` set and never calls `connection.UnsubscribeAsync`, so the server keeps sending messages that the client discards.
On subscribe failure, attempt `connection.UnsubscribeAsync` to clean up the server-side group subscription before rethrowing.
</issue_to_address>
### Comment 3
<location path="src/extensions/BootstrapBlazor.SignalR.Client/SignalRConnection.cs" line_range="64" />
<code_context>
+ {
+ _logger = logger;
+ _connection = new HubConnectionBuilder()
+ .WithUrl(navigation.ToAbsoluteUri(hubPath))
+ .WithAutomaticReconnect(new ConnectionRetryPolicy())
+ .Build();
</code_context>
<issue_to_address>
**Bearer-token clients cannot connect**
When the Hub requires bearer authentication and the WebAssembly app does not use same-origin cookies, `WithUrl` has no access-token provider or other authentication configuration, so the handshake lacks a bearer token and the Hub rejects the connection, leaving dispatch and related calls unusable.
Configure the Hub connection with an access-token provider or other bearer-token authentication options.
</issue_to_address>
### Comment 4
<location path="src/extensions/BootstrapBlazor.SignalR.Client/SignalRConnection.cs" line_range="204" />
<code_context>
+ try
+ {
+ await EnsureConnectedAsync(token);
+ await _connection.InvokeAsync("Dispatch", entryType, entry, token);
+ }
+ finally
</code_context>
<issue_to_address>
**Dispatch calls can hang**
When a Hub dispatch or server-side publish callback stops responding while the connection remains open, `SignalRConnection.DispatchAsync` invokes the Hub without a timeout, and the default cancellation token is not canceled. If the server-side publish callback or transport stops responding without disconnecting, the dispatch task stays pending and the caller never gets a result.
Apply a bounded timeout to dispatch invocations, while still allowing callers to provide cancellation.
</issue_to_address>
### Comment 5
<location path="src/extensions/BootstrapBlazor.SignalR.Client/DefaultDispatchService.cs" line_range="37-39" />
<code_context>
+ /// <param name="entry"><para lang="zh">消息分发项</para><para lang="en">The dispatch entry</para></param>
+ /// <param name="cancellationToken"><para lang="zh">取消令牌</para><para lang="en">The cancellation token</para></param>
+ /// <returns><para lang="zh">表示异步操作的任务</para><para lang="en">A task representing the asynchronous operation</para></returns>
+ public async Task DispatchAsync(DispatchEntry<TEntry> entry, CancellationToken cancellationToken = default)
+ {
+ await connection.DispatchAsync(_entryType, entry, cancellationToken);
+ await DispatchCoreAsync(entry, cancellationToken);
+ }
</code_context>
<issue_to_address>
**Retried dispatches publish twice**
When the server processes a dispatch but the response is lost and the caller retries after seeing a failure, `DispatchAsync` has no dispatch identifier or deduplication check, so if the server processes `Dispatch` but its response is lost, a caller retry publishes the same entry again to other clients and server-side subscribers.
Add a stable dispatch ID and deduplicate it server-side before invoking subscribers or broadcasting.
</issue_to_address>
### Comment 6
<location path="src/extensions/BootstrapBlazor.SignalR.Client/DefaultDispatchService.cs" line_range="39-40" />
<code_context>
+ /// <returns><para lang="zh">表示异步操作的任务</para><para lang="en">A task representing the asynchronous operation</para></returns>
+ public async Task DispatchAsync(DispatchEntry<TEntry> entry, CancellationToken cancellationToken = default)
+ {
+ await connection.DispatchAsync(_entryType, entry, cancellationToken);
+ await DispatchCoreAsync(entry, cancellationToken);
+ }
+
</code_context>
<issue_to_address>
**Sender misses committed dispatch**
When the caller cancels after the server processes the dispatch but before local notification finishes, `connection.DispatchAsync` can complete the server-side dispatch before cancellation is observed. The call then throws or `DispatchCoreAsync` exits on the canceled token, so other recipients receive the entry while this client's local subscribers do not.
Define post-commit cancellation semantics and ensure a committed dispatch is still delivered to local subscribers, or use an idempotent dispatch protocol that can safely reconcile the outcome.
</issue_to_address>
### Comment 7
<location path="src/extensions/BootstrapBlazor.SignalR.Client/DefaultDispatchService.cs" line_range="37-40" />
<code_context>
+ /// <param name="entry"><para lang="zh">消息分发项</para><para lang="en">The dispatch entry</para></param>
+ /// <param name="cancellationToken"><para lang="zh">取消令牌</para><para lang="en">The cancellation token</para></param>
+ /// <returns><para lang="zh">表示异步操作的任务</para><para lang="en">A task representing the asynchronous operation</para></returns>
+ public async Task DispatchAsync(DispatchEntry<TEntry> entry, CancellationToken cancellationToken = default)
+ {
+ await connection.DispatchAsync(_entryType, entry, cancellationToken);
+ await DispatchCoreAsync(entry, cancellationToken);
+ }
+
</code_context>
<issue_to_address>
**Local dispatches arrive out of order**
When two callers dispatch concurrently and the first caller is delayed between the server response and its local callback notification, `SignalRConnection.DispatchAsync` releases its gate when the server invocation completes, before `DispatchCoreAsync` notifies local subscribers. A second caller can therefore complete its server invocation and notify local subscribers before the first caller does, so local consumers observe entries in an order different from the server’s accepted order.
Serialize the complete remote-send and local-notification sequence per dispatch service, or assign ordering information and make local delivery preserve server acceptance order.
</issue_to_address>
### Comment 8
<location path="src/extensions/BootstrapBlazor.SignalR.Client/SignalRConnection.cs" line_range="203-204" />
<code_context>
+ await _gate.WaitAsync(token);
+ try
+ {
+ await EnsureConnectedAsync(token);
+ await _connection.InvokeAsync("Dispatch", entryType, entry, token);
+ }
+ finally
</code_context>
<issue_to_address>
**Idle Hub connections stay open**
When a caller dispatches without subscriptions or a connection service retaining the Hub, `EnsureConnectedAsync` starts the Hub, but `DispatchAsync` releases no usage reference and does not stop the connection, leaving the WebSocket open indefinitely.
Track dispatch-only connection use and stop the Hub after dispatch when no other consumer retains it.
</issue_to_address>
ArgoZhang
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #1256
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a typed connection snapshot response for authorized online connection queries.
New Features:
Enhancements: