Improve SignalR method invoke in bit Boilerplate project template (#11066) #11067
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Server as DiagnosticsController
participant Client as SignalR Client
Server->>Client: InvokeAsync<bool>("ShowMessageWithCustomAction", ...)
Client-->>Server: true/false
alt Client returns false
Server->>Client: SendAsync("ShowSimpleMessage", ...)
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the diagnostics controller to invoke a client-side SignalR method and fall back to a simple message if the client cannot perform the custom action.
- Replace
SendAsyncwithInvokeAsync<bool>to capture success feedback - Add fallback
SendAsyncwhenInvokeAsyncreturnsfalse
Comments suppressed due to low confidence (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs:62
- The new fallback branch isn't covered by existing tests. Add a unit or integration test that simulates a
falsereturn fromInvokeAsyncto verify the simple message is sent.
if (success is false) // Client would return false if it's unable to show the message with custom action.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs:61
- [nitpick] The variable name
successis generic. Consider renaming it to something more descriptive, likeisMessageDisplayedorinvokeSucceeded, to improve readability.
var success = await appHubContext.Clients.Client(signalRConnectionId).InvokeAsync<bool>(SignalREvents.SHOW_MESSAGE, $"Open terms page. {DateTimeOffset.Now:HH:mm:ss}", new { pageUrl = Urls.TermsPage, action = "testAction" }, cancellationToken);
| if (string.IsNullOrEmpty(signalRConnectionId) is false) | ||
| { | ||
| await appHubContext.Clients.Client(signalRConnectionId).SendAsync(SignalREvents.SHOW_MESSAGE, $"Open terms page. {DateTimeOffset.Now:HH:mm:ss}", new { pageUrl = Urls.TermsPage, action = "testAction" }, cancellationToken); | ||
| var success = await appHubContext.Clients.Client(signalRConnectionId).InvokeAsync<bool>(SignalREvents.SHOW_MESSAGE, $"Open terms page. {DateTimeOffset.Now:HH:mm:ss}", new { pageUrl = Urls.TermsPage, action = "testAction" }, cancellationToken); |
There was a problem hiding this comment.
The call to InvokeAsync<bool> can throw if the client method isn't implemented or the connection is lost. Consider wrapping it in a try-catch to handle exceptions and trigger the fallback scenario.
| { | ||
| await appHubContext.Clients.Client(signalRConnectionId).SendAsync(SignalREvents.SHOW_MESSAGE, $"Open terms page. {DateTimeOffset.Now:HH:mm:ss}", new { pageUrl = Urls.TermsPage, action = "testAction" }, cancellationToken); | ||
| var success = await appHubContext.Clients.Client(signalRConnectionId).InvokeAsync<bool>(SignalREvents.SHOW_MESSAGE, $"Open terms page. {DateTimeOffset.Now:HH:mm:ss}", new { pageUrl = Urls.TermsPage, action = "testAction" }, cancellationToken); | ||
| if (success is false) // Client would return false if it's unable to show the message with custom action. |
There was a problem hiding this comment.
[nitpick] Using success is false works, but the more conventional idiom is if (!success). Switching to !success can make the code more immediately familiar to readers.
| if (success is false) // Client would return false if it's unable to show the message with custom action. | |
| if (!success) // Client would return false if it's unable to show the message with custom action. |
1 participant
closes #11066
Summary by CodeRabbit