fix: loops when processing event from staging area

[sampaiodiego]

Summary by CodeRabbit

• Bug Fixes

  • More accurate authorization errors by only failing when required auth events are truly missing.
  • Consistent success/failure signal when fetching missing events, improving reliability.
  • Clearer error logs during event processing, aiding troubleshooting.

• Refactor

  • Parallelized fetching of missing events for faster processing and reduced latency.
  • Streamlined event staging behavior to reduce unnecessary operations.

[coderabbitai]

Walkthrough

The PR removes getNextStagedEventForRoom from EventStagingRepository and EventService, changes MissingEventService.fetchMissingEvent to return Promise, and updates StagingAreaService to parallelize missing-event fetching, refine authorization error conditions, and log payloads while marking events as unstaged on errors.

Changes

Cohort / File(s)	Summary
API removal: next staged event packages/federation-sdk/src/repositories/event-staging.repository.ts, packages/federation-sdk/src/services/event.service.ts	Removed public method `getNextStagedEventForRoom(roomId: string): Promise<EventStagingStore
Missing events return semantics packages/federation-sdk/src/services/missing-event.service.ts	fetchMissingEvent now returns Promise<boolean>; all paths return boolean: true when already exists or successfully processed; false on fetch failure or exceptions.
Staging area processing adjustments packages/federation-sdk/src/services/staging-area.service.ts	Error logging includes event payload and calls markEventAsUnstaged(event); missing-event fetches run in parallel via Promise.all; authorization error triggered only if missing events were added and at least one is an auth event.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant SA as StagingAreaService
  participant MS as MissingEventService
  participant ES as EventService
  participant L as Logger

  C->>SA: process(event)
  SA->>SA: identify dependencies (missing, authEvents)
  par Parallel fetch of missing events
    SA->>MS: fetchMissingEvent(dep1)
    SA->>MS: fetchMissingEvent(dep2)
    SA->>MS: fetchMissingEvent(...deps)
  end
  MS-->>SA: boolean results[]
  SA->>SA: addedMissing = results contains true
  alt Added missing auth event
    SA->>SA: check authEvents ∩ missing
    SA-->>C: throw MissingAuthorizationEventsError
  else No blocking auth condition
    SA-->>C: continue processing
  end

  rect rgba(255,230,230,0.5)
    note over SA,ES: Error path
    SA->>L: log "Error processing event" with payload
    SA->>ES: markEventAsUnstaged(event)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: missing events not being processed properly #212 — Also modifies event staging APIs around selecting staged events (adds/adjusts least-depth retrieval), closely related to the removal of next-staged retrieval in this PR.

Suggested reviewers

• rodrigok
• ggazzo

Poem

A nibble of code, a hop through the queue,
I fetched all the missing—now parallel, too!
If auth is astray, I thump with care,
Unstage the event, with logs laid bare.
One less method, lean and bright—
Carrots compiled, the build feels light. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit's high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the key change of preventing infinite loops in the staging-area processing logic, which is the primary focus of the modifications in staging-area.service.ts and aligns with the branch’s intent.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch prevent-loops-in-stagin-area

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f986212 and daf03e8.

📒 Files selected for processing (4)

• packages/federation-sdk/src/repositories/event-staging.repository.ts (0 hunks)
• packages/federation-sdk/src/services/event.service.ts (1 hunks)
• packages/federation-sdk/src/services/missing-event.service.ts (3 hunks)
• packages/federation-sdk/src/services/staging-area.service.ts (2 hunks)

💤 Files with no reviewable changes (1)

• packages/federation-sdk/src/repositories/event-staging.repository.ts

🧰 Additional context used

🧬 Code graph analysis (1)

packages/federation-sdk/src/services/staging-area.service.ts (1)

packages/room/src/manager/event-wrapper.ts (1)

• event (106-115)

🔇 Additional comments (6)

packages/federation-sdk/src/services/missing-event.service.ts (1)

25-62: LGTM! Boolean return values consistently signal success/failure.

The method signature change to Promise<boolean> is implemented correctly across all execution paths:

• Returns true when the event already exists (line 33)
• Returns false when fetching fails (line 46)
• Returns true after successful processing (line 56)
• Returns false in the error handler (line 61)

This allows callers to react appropriately to missing-event fetch outcomes.

packages/federation-sdk/src/services/event.service.ts (2)

817-821: LGTM! Comment formatting improved.

The addition of blank lines improves readability of the JSDoc comment for getBackfillEvents.

---

1-867: No remaining references to getNextStagedEventForRoom
Search for getNextStagedEventForRoom returned no hits—removal is safe.

packages/federation-sdk/src/services/staging-area.service.ts (3)

82-89: LGTM! Enhanced error handling prevents stuck events.

The changes improve error handling by:

1. Logging the full event payload (line 84) for better debugging context
2. Explicitly marking events as unstaged (line 88) when processing fails

This prevents events from remaining in the staging area indefinitely, which directly addresses the PR objective of fixing loops when processing events from the staging area.

---

134-148: LGTM! Parallelized fetching improves performance.

The refactor from sequential to parallel missing-event fetching using Promise.all (lines 134-146) is a good performance improvement when multiple dependencies are missing. The boolean return values from fetchMissingEvent enable the addedMissing check (line 148) to determine if at least one fetch succeeded.

---

150-154: LGTM! Tightened authorization check reduces false postponements.

The condition at line 152 now requires both:

1. At least one missing event was successfully fetched (addedMissing)
2. At least one of those missing events is an auth event

This prevents unnecessary postponement when either:

• No missing events were fetched successfully, OR
• The missing events don't include any auth events

This refinement helps prevent loops by avoiding unnecessary event re-queuing.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.84%. Comparing base (f986212) to head (daf03e8).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #246   +/-   ##
=======================================
  Coverage   81.84%   81.84%           
=======================================
  Files          63       63           
  Lines        4705     4705           
=======================================
  Hits         3851     3851           
  Misses        854      854           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.