feat(indexer): add handler registry, stream lifecycle handlers, distribution event handlers#48
Conversation
…ibution event handlers issue Fundable-Protocol#30 — indexer/common/src/handlers/ - HandlerRegistry interface with register/match/dispatch API - Filter by contractId, eventName, or both; empty filter matches all - EventHandlerRegistry implementation: sequential dispatch, error captured per handler - 15 unit tests: matching rules, ordering, error isolation, idempotency issue Fundable-Protocol#35 — indexer/streams/src/handlers/ - StreamFunded, StreamWithdrawal, StreamCancelled handlers - Shared utils: parseEventData, requireStringField, requireTopic, getEventIdentity - Each handler: parse → validate → map → identity following streamCreated.ts pattern - 24 unit tests: valid parse, wrong topic, missing/empty fields, record shape, idempotency issue Fundable-Protocol#38 — indexer/distributions/src/handlers/ - DistributionCreated, TokensClaimed, BatchPaused, BatchResumed handlers - requireIntField validator for recipientCount (integer, not string) - Record types align with DistributionBatch + ClaimAction entity schema (PR Fundable-Protocol#44) - 26 unit tests: happy path, wrong topics, missing fields, identity uniqueness, idempotency closes Fundable-Protocol#30 closes Fundable-Protocol#35 closes Fundable-Protocol#38
|
@pre-cious-Igwealor Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughAdds a shared handler registry API and shared event/record types, then implements stream and distribution event handlers that parse payloads, map records, derive event identities, and validate the behavior with Vitest tests. ChangesShared handler registry
Stream lifecycle handlers
Distribution event handlers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
indexer/streams/src/handlers/stream-lifecycle.test.ts (1)
56-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGood coverage; consider one more parse case.
These tests exercise invalid JSON and missing/empty/wrong-type fields well. If you adopt the non-object-JSON guard suggested in
utils.ts, add a case whereevent.data = "null"to lock in the clean validation error rather than a rawTypeError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indexer/streams/src/handlers/stream-lifecycle.test.ts` around lines 56 - 72, The parseStreamFundedPayload tests cover invalid JSON and missing fields, but they should also lock in the non-object JSON guard from utils.ts. Add a test case in stream-lifecycle.test.ts where event.data is set to "null" and assert the same clean validation error path is thrown instead of allowing a raw TypeError, using parseStreamFundedPayload and makeEvent as the entry points.indexer/streams/src/handlers/utils.ts (1)
1-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated helpers across
streamsanddistributions.
getEventIdentity,parseEventData,requireStringField, andrequireTopichere are essentially identical toindexer/distributions/src/handlers/utils.ts. Since@fundable-indexer/commonis already a workspace dependency, these generic helpers are a good candidate to hoist intocommon(parameterized by the event identity fields) to prevent the two copies from diverging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indexer/streams/src/handlers/utils.ts` around lines 1 - 37, The helper functions in this module are duplicated in the distributions handlers, so move the shared logic from getEventIdentity, parseEventData, requireStringField, and requireTopic into `@fundable-indexer/common` and update both callers to use the shared implementation. Make getEventIdentity generic enough to work across both event shapes by parameterizing the identity fields, then keep the streams-specific wrapper or direct import here aligned with the common version so the two copies cannot diverge.indexer/streams/src/handlers/types.ts (1)
3-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
StreamEventfield redeclarations.StreamEventrepeats fields already declared onSorobanEvent; if no extra fields are needed, use the base type directly or make this a type alias.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indexer/streams/src/handlers/types.ts` around lines 3 - 10, The StreamEvent definition is redundantly redeclaring fields already inherited from SorobanEvent, so simplify the type in types.ts by removing the duplicate property list. If StreamEvent adds no new shape beyond SorobanEvent, replace the interface with a direct type alias to SorobanEvent or otherwise rely on the base type; keep the change centered on the StreamEvent symbol so downstream handlers continue using the same event contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@indexer/common/src/handlers/types.ts`:
- Around line 17-23: The shared SorobanEvent contract is inconsistent with how
eventName filtering works: SorobanEvent.topics is documented as raw XDR, but the
registry and tests assume topics[0] is the decoded event name used by eventName
filtering. Update the shared type and its related handler/registry logic
(SorobanEvent, eventName matching, and any tests) so they all agree on whether
topics contains decoded names or encoded values, and make the filter compare
against the same representation everywhere.
In `@indexer/distributions/src/handlers/utils.ts`:
- Around line 7-13: The parseEventData helper is only guarding invalid JSON, but
it still allows valid non-object payloads like null, arrays, or primitives to
pass through and later break requireStringField/requireIntField with uncaught
TypeError. Update parseEventData in utils.ts to validate the parsed value is a
non-null plain object before returning it, and throw the same validation error
path when the payload is not an object so downstream field access stays safe.
In `@indexer/streams/src/handlers/utils.ts`:
- Around line 7-13: parseEventData currently accepts any valid JSON and can
return non-object values like null or primitives, which later breaks
requireStringField with a raw TypeError. Update parseEventData in utils.ts to
validate that JSON.parse(event.data) returns a non-null object before casting,
and throw a clear validation error when the parsed value is not an object; keep
the invalid-JSON error path distinct from the shape check.
---
Nitpick comments:
In `@indexer/streams/src/handlers/stream-lifecycle.test.ts`:
- Around line 56-72: The parseStreamFundedPayload tests cover invalid JSON and
missing fields, but they should also lock in the non-object JSON guard from
utils.ts. Add a test case in stream-lifecycle.test.ts where event.data is set to
"null" and assert the same clean validation error path is thrown instead of
allowing a raw TypeError, using parseStreamFundedPayload and makeEvent as the
entry points.
In `@indexer/streams/src/handlers/types.ts`:
- Around line 3-10: The StreamEvent definition is redundantly redeclaring fields
already inherited from SorobanEvent, so simplify the type in types.ts by
removing the duplicate property list. If StreamEvent adds no new shape beyond
SorobanEvent, replace the interface with a direct type alias to SorobanEvent or
otherwise rely on the base type; keep the change centered on the StreamEvent
symbol so downstream handlers continue using the same event contract.
In `@indexer/streams/src/handlers/utils.ts`:
- Around line 1-37: The helper functions in this module are duplicated in the
distributions handlers, so move the shared logic from getEventIdentity,
parseEventData, requireStringField, and requireTopic into
`@fundable-indexer/common` and update both callers to use the shared
implementation. Make getEventIdentity generic enough to work across both event
shapes by parameterizing the identity fields, then keep the streams-specific
wrapper or direct import here aligned with the common version so the two copies
cannot diverge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 198df6f4-023f-4407-9e06-75afe3629abd
📒 Files selected for processing (16)
indexer/common/src/handlers/registry.test.tsindexer/common/src/handlers/registry.tsindexer/common/src/handlers/types.tsindexer/common/src/index.tsindexer/distributions/src/handlers/batchPause.tsindexer/distributions/src/handlers/distribution-events.test.tsindexer/distributions/src/handlers/distributionCreated.tsindexer/distributions/src/handlers/tokensClaimed.tsindexer/distributions/src/handlers/types.tsindexer/distributions/src/handlers/utils.tsindexer/streams/src/handlers/stream-lifecycle.test.tsindexer/streams/src/handlers/streamCancel.tsindexer/streams/src/handlers/streamFunded.tsindexer/streams/src/handlers/streamWithdrawal.tsindexer/streams/src/handlers/types.tsindexer/streams/src/handlers/utils.ts
| /** Minimal shape of a Soroban event delivered to handlers. */ | ||
| export interface SorobanEvent extends SorobanEventIdentity { | ||
| /** Array of XDR-encoded topics. The first element is conventionally the event name. */ | ||
| topics: string[]; | ||
| /** JSON-encoded event data. */ | ||
| data: string; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the shared topic contract with eventName filtering.
SorobanEvent.topics is documented as XDR-encoded here, but the registry and tests treat topics[0] as a decoded name like "StreamCreated". If dispatch receives raw Soroban topics, every eventName filter will miss. Either expose decoded topics/event names in the shared event contract or make the filter explicitly operate on the encoded value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@indexer/common/src/handlers/types.ts` around lines 17 - 23, The shared
SorobanEvent contract is inconsistent with how eventName filtering works:
SorobanEvent.topics is documented as raw XDR, but the registry and tests assume
topics[0] is the decoded event name used by eventName filtering. Update the
shared type and its related handler/registry logic (SorobanEvent, eventName
matching, and any tests) so they all agree on whether topics contains decoded
names or encoded values, and make the filter compare against the same
representation everywhere.
| export function parseEventData(event: DistributionEvent): Record<string, unknown> { | ||
| try { | ||
| return JSON.parse(event.data) as Record<string, unknown>; | ||
| } catch { | ||
| throw new Error("Failed to parse event data: invalid JSON"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against non-object JSON payloads.
JSON.parse can return null (or a primitive/array) for valid JSON that isn't an object. When it returns null, the subsequent parsed[field] access in requireStringField/requireIntField throws an uncaught TypeError ("Cannot read properties of null") instead of the intended validation error, defeating the clean error path here.
🛡️ Proposed guard
export function parseEventData(event: DistributionEvent): Record<string, unknown> {
+ let parsed: unknown;
try {
- return JSON.parse(event.data) as Record<string, unknown>;
+ parsed = JSON.parse(event.data);
} catch {
throw new Error("Failed to parse event data: invalid JSON");
}
+ if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
+ throw new Error("Failed to parse event data: expected a JSON object");
+ }
+ return parsed as Record<string, unknown>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function parseEventData(event: DistributionEvent): Record<string, unknown> { | |
| try { | |
| return JSON.parse(event.data) as Record<string, unknown>; | |
| } catch { | |
| throw new Error("Failed to parse event data: invalid JSON"); | |
| } | |
| } | |
| export function parseEventData(event: DistributionEvent): Record<string, unknown> { | |
| let parsed: unknown; | |
| try { | |
| parsed = JSON.parse(event.data); | |
| } catch { | |
| throw new Error("Failed to parse event data: invalid JSON"); | |
| } | |
| if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { | |
| throw new Error("Failed to parse event data: expected a JSON object"); | |
| } | |
| return parsed as Record<string, unknown>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@indexer/distributions/src/handlers/utils.ts` around lines 7 - 13, The
parseEventData helper is only guarding invalid JSON, but it still allows valid
non-object payloads like null, arrays, or primitives to pass through and later
break requireStringField/requireIntField with uncaught TypeError. Update
parseEventData in utils.ts to validate the parsed value is a non-null plain
object before returning it, and throw the same validation error path when the
payload is not an object so downstream field access stays safe.
| export function parseEventData(event: StreamEvent): Record<string, unknown> { | ||
| try { | ||
| return JSON.parse(event.data) as Record<string, unknown>; | ||
| } catch { | ||
| throw new Error("Failed to parse event data: invalid JSON"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
parseEventData doesn't guard non-object JSON.
JSON.parse succeeds for valid-but-non-object JSON like null, 42, or "abc". For null, the subsequent parsed[field] access in requireStringField throws a raw TypeError ("Cannot read properties of null") instead of the intended validation error. Consider asserting the parsed value is a non-null object.
🛡️ Proposed guard
export function parseEventData(event: StreamEvent): Record<string, unknown> {
try {
- return JSON.parse(event.data) as Record<string, unknown>;
+ const parsed = JSON.parse(event.data);
+ if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
+ throw new Error("Failed to parse event data: expected a JSON object");
+ }
+ return parsed as Record<string, unknown>;
} catch {
throw new Error("Failed to parse event data: invalid JSON");
}
}Note: the throw inside try is recaught here; if you want the object-shape error distinct from the JSON error, move the shape check outside the try.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function parseEventData(event: StreamEvent): Record<string, unknown> { | |
| try { | |
| return JSON.parse(event.data) as Record<string, unknown>; | |
| } catch { | |
| throw new Error("Failed to parse event data: invalid JSON"); | |
| } | |
| } | |
| export function parseEventData(event: StreamEvent): Record<string, unknown> { | |
| try { | |
| const parsed = JSON.parse(event.data); | |
| if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { | |
| throw new Error("Failed to parse event data: expected a JSON object"); | |
| } | |
| return parsed as Record<string, unknown>; | |
| } catch { | |
| throw new Error("Failed to parse event data: invalid JSON"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@indexer/streams/src/handlers/utils.ts` around lines 7 - 13, parseEventData
currently accepts any valid JSON and can return non-object values like null or
primitives, which later breaks requireStringField with a raw TypeError. Update
parseEventData in utils.ts to validate that JSON.parse(event.data) returns a
non-null object before casting, and throw a clear validation error when the
parsed value is not an object; keep the invalid-JSON error path distinct from
the shape check.
1 participant
Summary
issue #30 — Handler registration interface (
indexer/common/src/handlers/)types.ts—SorobanEvent,HandlerResult,EventHandler<T>,HandlerFilter,HandlerEntry,HandlerRegistryinterfaceregistry.ts—EventHandlerRegistryimplementation: filter bycontractId,eventName, or both; empty filter matches all; sequentialdispatchwith per-handler error capturecommon/src/index.tsso domain packages import without coupling to internalsissue #35 — Stream funding, withdrawal, and cancel handlers (
indexer/streams/src/handlers/)streamFunded.ts—StreamFundedevent handlerstreamWithdrawal.ts—StreamWithdrawalevent handlerstreamCancel.ts—StreamCancelledevent handlerutils.ts— sharedparseEventData,requireStringField,requireTopic,getEventIdentitystreamCreated.ts(PR feat: implement StreamCreated handler #43)issue #38 — Distribution event handlers (
indexer/distributions/src/handlers/)distributionCreated.ts—DistributionCreatedhandler (includesrecipientCount: numbervalidated as integer)tokensClaimed.ts—TokensClaimedhandlerbatchPause.ts—BatchPausedandBatchResumedhandlersDistributionBatch+ClaimActionentity schema (PR feat(indexer/distributions): define distributions database schema #44)closes #30
closes #35
closes #38
Summary by CodeRabbit
New Features
Bug Fixes