feat: Add stream client to dotnet sdk - #156
Open
aholmis wants to merge 8 commits into
Open
Conversation
aholmis
changed the title
aholmis
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a new .NET streaming client for consuming Heimdall Cloud Stream API Server-Sent Events, along with supporting auth/header infrastructure, tests, and documentation/examples to help SDK consumers adopt streaming.
Changes:
- Introduces
HeimdallStreamClient/IHeimdallStreamClientto receive SSE events with automatic reconnect + backoff and token refresh. - Refactors shared auth/client header handling into
AccessTokenHeaderRefresher, and centralizes endpoint constants inHeimdallApiEndpoints. - Adds unit tests for parsing/auth/reconnect behavior, plus README + example console app.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenStreaming/WhenReconnecting.cs | Unit tests for reconnect and cancellation behavior. |
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenStreaming/WhenParsingEvents.cs | Unit tests for SSE parsing, skipping heartbeats/unknown events, and recovery from malformed payloads. |
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenStreaming/WhenAuthenticating.cs | Unit tests for header attachment, token reuse, and forced refresh on 401. |
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenStreaming/Fakes/SseTestData.cs | SSE test-body and canned HTTP response builders. |
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenStreaming/Fakes/NeverEndingSseStream.cs | Test stream for cancellation behavior during long-lived reads. |
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenStreaming/Fakes/CountingAccessTokenProvider.cs | Token provider fake that counts acquisitions for assertions. |
| dotnet/tests/unit/HeimdallPower.Api.Client.UnitTests/WhenComputingRetryDelay/WhenComputingRetryDelay.cs | Unit tests for exponential backoff + jitter math. |
| dotnet/README.md | Adds streaming documentation and DI registration guidance. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/StreamConnectionRetryPolicy.cs | Backoff + jitter policy used by stream reconnect logic. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/IHeimdallStreamClient.cs | Public interface for receiving stream events. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/HeimdallStreamJsonSerializerOptions.cs | Shared JSON serializer options for stream payloads (snake_case, enums). |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/HeimdallStreamClient.cs | Core SSE client implementation (connect, parse, reconnect, token refresh). |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/HeimdallEventEnvelope.cs | Event envelope model with typed accessor for DLR events. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/CapacityMonitoring/Lines/HeimdallDlrEvent.cs | DLR payload model and metric name constant. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/HeimdallApiHttpClient.cs | Refactors REST client to use shared header/token refresher. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/HeimdallApiEndpoints.cs | Centralizes API/stream URLs + AAD/B2C authority/scope constants. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/HeimdallApiClient.cs | Switches to shared endpoint constants. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/ClientHeaders.cs | Centralizes x-client-* header construction. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/AccessTokenHeaderRefresher.cs | New shared component to keep auth/client headers fresh with concurrency control. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client.Extensions/HeimdallStreamClientOptions.cs | DI options for stream client credentials/proxy/metadata. |
| dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client.Extensions/HeimdallStreamClientExtensions.cs | Adds AddHeimdallPowerStreamClient DI registration. |
| dotnet/HeimdallPower.Api.Client.slnx | Adds the new stream example project to the solution. |
| dotnet/examples/Stream.Client.Examples/Stream.Client.Examples.csproj | New example project for stream usage. |
| dotnet/examples/Stream.Client.Examples/Program.cs | Example showing how to consume stream events and stop via Ctrl+C. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/AccessTokenHeaderRefresher.cs:28
SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken)returnsbool(timeout vs lock-acquired). The current code ignores the return value, so a timeout could lead to executing the critical section without having entered the semaphore and then callingRelease(), which can throwSemaphoreFullExceptionand break token refresh under contention.
await _tokenLock.WaitAsync(TimeSpan.FromSeconds(30), cancellationToken);
There was a problem hiding this comment.
🟡 Changes recommended
Retry timing, graceful cancellation, and token-refresh synchronization contain correctness issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/Stream/StreamConnectionRetryPolicy.cs:48
- Jitter is applied after capping, so this can return up to 120% of
MaxDelay. That contradicts the public option's guarantee thatMaxDelayis the maximum delay between attempts. Apply the cap again after jitter (the new tests should also assertdelay <= MaxDelay, notMaxDelay * 1.2).
dotnet/HeimdallPower.Api.Client/HeimdallPower.Api.Client/AccessTokenHeaderRefresher.cs:28
SemaphoreSlim.WaitAsync(TimeSpan, ...)returnsfalseon timeout rather than throwing. Ignoring that result lets a timed-out caller enter the critical section without owning the semaphore and then callRelease(), allowing concurrent token/header mutation and potentially causingSemaphoreFullExceptionwhen the real owner releases. Wait with cancellation only, or check the Boolean and fail on timeout.
await _tokenLock.WaitAsync(TimeSpan.FromSeconds(30), cancellationToken);
- Files reviewed: 27/27 changed files
- Comments generated: 4
- Review effort level: Balanced
| /// <summary> | ||
| /// Controls which physical quantity is returned by a rating endpoint. | ||
| /// </summary> | ||
| public enum Quantity |
| { | ||
| failedAttempts++; | ||
| infoLogger?.Invoke($"Stream error: unauthorized. Refreshing token and reconnecting... (attempt #{failedAttempts})"); | ||
| await _tokenRefresher.ForceRefreshAsync(token); |
| { | ||
| if (_retryPolicy.ShouldRetry(failedAttempts)) | ||
| { | ||
| await Task.Delay(_retryPolicy.GetDelay(failedAttempts), token); |
|
|
||
| Assert.Single(events); | ||
| Assert.Contains(traceMessages, m => m.Contains("Heartbeat", StringComparison.OrdinalIgnoreCase)); | ||
| Assert.Contains(logMessages, m => !m.Contains("Heartbeat", StringComparison.OrdinalIgnoreCase)); |
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.
POWER-5102
Add C# .NET client to receive streaming events from Heimdall Cloud Stream Api