Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: Resgrid/Core/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. Your free on-demand review promotion remains active until October 9, 2026 at 6:00 PM UTC. 📝 WalkthroughWalkthroughThe hubs now check connection cancellation before selected group operations and notifications. Geolocation tracking accepts the connection-aborted token and can return null, causing its callers to stop before further subscription work. ChangesConnection cancellation handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The identified group-operation concern does not warrant a change, and no actionable issue introduced by this PR remains. The change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
| public async Task Open_connection_changes_groups(Func<Harness, Task> arrange, Func<Harness, Task> act) | ||
| { | ||
| var harness = new Harness(); | ||
| await arrange(harness); |
There was a problem hiding this comment.
Unhandled task rejection in await arrange(harness) prevents arrangement failures from being reported with test context. Catch Exception and call Assert.Fail with the failure details in Tests/Resgrid.Tests/Web/Eventing/ClosedConnectionHubTests.cs:57, :66, and :70, and Tests/Resgrid.Tests/Web/Eventing/GeolocationVisibilityTests.cs:192 and :194.
Kody rule violation: Handle async operations with proper error handling
try { await arrange(harness); } catch (Exception exception) { Assert.Fail($"Arrange failed: {exception}"); }Prompt for LLM
File Tests/Resgrid.Tests/Web/Eventing/ClosedConnectionHubTests.cs:
Line 54:
Unhandled task rejection in `await arrange(harness)` prevents arrangement failures from being reported with test context. Catch `Exception` and call `Assert.Fail` with the failure details in `Tests/Resgrid.Tests/Web/Eventing/ClosedConnectionHubTests.cs:57`, `:66`, and `:70`, and `Tests/Resgrid.Tests/Web/Eventing/GeolocationVisibilityTests.cs:192` and `:194`.
Suggested Code:
try { await arrange(harness); } catch (Exception exception) { Assert.Fail($"Arrange failed: {exception}"); }
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| public sealed class Harness | ||
| { | ||
| private readonly CancellationTokenSource _connectionAborted = new CancellationTokenSource(); |
There was a problem hiding this comment.
Resource leak occurs because the CancellationTokenSource stored in _connectionAborted remains undisposed after the test completes. Implement IDisposable on Harness and dispose _connectionAborted during teardown.
Kody rule violation: Use using statements for disposable resources
private readonly CancellationTokenSource _connectionAborted = new CancellationTokenSource();
public void Dispose() => _connectionAborted.Dispose();Prompt for LLM
File Tests/Resgrid.Tests/Web/Eventing/ClosedConnectionHubTests.cs:
Line 80:
Resource leak occurs because the `CancellationTokenSource` stored in `_connectionAborted` remains undisposed after the test completes. Implement `IDisposable` on `Harness` and dispose `_connectionAborted` during teardown.
Suggested Code:
private readonly CancellationTokenSource _connectionAborted = new CancellationTokenSource();
public void Dispose() => _connectionAborted.Dispose();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Summary
Fixes SignalR operations that continue executing after a client connection has closed.
Changes
This prevents SignalR and the Redis backplane from waiting for acknowledgements from connections that are no longer held by the server.