Change IEventReader to return IAsyncEnumerable#522
Conversation
…d arrays The interface methods ReadEvents and ReadEventsBackwards now return IAsyncEnumerable<StreamEvent> without the failIfNotFound parameter. The previous signatures are preserved as extension methods on IEventReader so all downstream callers continue to work unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review Summary by QodoChange IEventReader to return IAsyncEnumerable instead of materialized arrays
WalkthroughsDescription• Changed IEventReader interface to return IAsyncEnumerable<StreamEvent> instead of Task<StreamEvent[]> • Removed failIfNotFound parameter from interface methods; now always throws StreamNotFound if stream missing • Added extension methods in StoreFunctions preserving old signatures for backward compatibility • Updated all implementations: KurrentDB, SQL, Redis, Elastic, InMemory, Tiered, and Traced decorators Diagramflowchart LR
A["IEventReader Interface"] -->|"Return type changed"| B["IAsyncEnumerable<StreamEvent>"]
A -->|"Parameter removed"| C["failIfNotFound removed"]
D["StoreFunctions Extensions"] -->|"Preserve old signatures"| E["Task<StreamEvent[]> with failIfNotFound"]
F["All Implementations"] -->|"Updated to new interface"| G["KurrentDB, SQL, Redis, Elastic, InMemory, Tiered, Traced"]
File Changes1. src/Core/src/Eventuous.Persistence/EventStore/IEventReader.cs
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 851a6d7282
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await foreach (var item in source.WithCancellation(cancellationToken).ConfigureAwait(false)) { | ||
| yield return item; | ||
| } | ||
|
|
||
| activity?.SetActivityStatus(ActivityStatus.Ok()); |
There was a problem hiding this comment.
Mark traced read failures in async enumeration path
TraceEnumerable never wraps the await foreach in a try/catch, so when the underlying reader throws during enumeration (for example StreamNotFound or deserialization/read exceptions), telemetry does not call measure.SetError() or set ActivityStatus.Error, unlike the existing Trace methods. This regresses observability for failed reads because failed operations can appear successful/unknown in traces and metrics.
Useful? React with 👍 / 👎.
src/Core/src/Eventuous.Persistence/EventStore/TieredEventReader.cs
Outdated
Show resolved
Hide resolved
src/Core/src/Eventuous.Persistence/EventStore/TieredEventReader.cs
Outdated
Show resolved
Hide resolved
Test Results 60 files + 40 60 suites +40 38m 51s ⏱️ + 27m 3s Results for commit 5df61c1. ± Comparison against base commit 49e0463. This pull request removes 5 and adds 15 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Use manual enumerator with try-catch around MoveNextAsync to properly set ActivityStatus.Error and measure.SetError() when the underlying async enumeration throws, matching the behavior of the existing Trace methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TieredEventReader: - Fix IndexOutOfRangeException when hot store returns no events by checking hotEvents.Length before accessing hotEvents[0] - Throw StreamNotFound when neither hot nor archive tier has events, instead of silently returning an empty sequence ElasticEventStore: - Use StreamExists to distinguish "no events in range" from "stream does not exist", fixing false StreamNotFound on final empty page when stream length is an exact multiple of the page size Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
IEventReader.ReadEventsandReadEventsBackwardsnow returnIAsyncEnumerable<StreamEvent>instead ofTask<StreamEvent[]>, with thefailIfNotFoundparameter removed from the interfacefailIfNotFound, returningTask<StreamEvent[]>) are preserved as extension methods inStoreFunctions, so all downstream callers compile and work unchangedTest plan
🤖 Generated with Claude Code