Conversation
Prefer feeds.stories_feed.updated, but fall back to getOrCreate when the WS event is intermittently dropped so CI does not fail spuriously. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughChangesStories watch synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🤖 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 `@packages/feeds-client/__integration-tests__/utils.ts`:
- Around line 124-134: Update the flow around feed.markActivity and eventPromise
to attach the existing fallback catch handler to the event waiter, then await
the API call and handled event promise concurrently with Promise.all. Preserve
the getOrCreate({ watch: true }) fallback when the event wait fails.
🪄 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: d3b68fb9-5c67-41f4-89e4-b2bd44052fed
📒 Files selected for processing (2)
packages/feeds-client/__integration-tests__/stories.test.tspackages/feeds-client/__integration-tests__/utils.ts
| const eventPromise = waitForEvent( | ||
| feed, | ||
| 'feeds.stories_feed.updated', | ||
| eventTimeoutMs, | ||
| ); | ||
| await feed.markActivity({ mark_watched: activityIds }); | ||
| try { | ||
| await eventPromise; | ||
| } catch { | ||
| await feed.getOrCreate({ watch: true }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Await the API promise and the event waiter together using Promise.all.
As per coding guidelines, integration tests that trigger API calls and wait for real-time events must await both the API promise and the event using Promise.all to prevent unhandled rejections.
In the current implementation, if feed.markActivity throws an error, the function will exit immediately, leaving eventPromise without a catch handler. When it subsequently times out, it will cause an UnhandledPromiseRejection and crash the test suite.
You can fix this by attaching a catch handler to eventPromise and awaiting both promises concurrently.
🛠️ Proposed fix
- const eventPromise = waitForEvent(
- feed,
- 'feeds.stories_feed.updated',
- eventTimeoutMs,
- );
- await feed.markActivity({ mark_watched: activityIds });
- try {
- await eventPromise;
- } catch {
+ const eventPromise = waitForEvent(
+ feed,
+ 'feeds.stories_feed.updated',
+ eventTimeoutMs,
+ ).catch(() => null);
+
+ const [, eventResult] = await Promise.all([
+ feed.markActivity({ mark_watched: activityIds }),
+ eventPromise,
+ ]);
+
+ if (eventResult === null) {
await feed.getOrCreate({ watch: true });
}📝 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.
| const eventPromise = waitForEvent( | |
| feed, | |
| 'feeds.stories_feed.updated', | |
| eventTimeoutMs, | |
| ); | |
| await feed.markActivity({ mark_watched: activityIds }); | |
| try { | |
| await eventPromise; | |
| } catch { | |
| await feed.getOrCreate({ watch: true }); | |
| } | |
| const eventPromise = waitForEvent( | |
| feed, | |
| 'feeds.stories_feed.updated', | |
| eventTimeoutMs, | |
| ).catch(() => null); | |
| const [, eventResult] = await Promise.all([ | |
| feed.markActivity({ mark_watched: activityIds }), | |
| eventPromise, | |
| ]); | |
| if (eventResult === null) { | |
| await feed.getOrCreate({ watch: true }); | |
| } |
🤖 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 `@packages/feeds-client/__integration-tests__/utils.ts` around lines 124 - 134,
Update the flow around feed.markActivity and eventPromise to attach the existing
fallback catch handler to the event waiter, then await the API call and handled
event promise concurrently with Promise.all. Preserve the getOrCreate({ watch:
true }) fallback when the event wait fails.
Source: Coding guidelines
1 participant
Summary
stories.test.tsmark-watched cases that timed out waiting forfeeds.stories_feed.updated(seen on CI run 29350895100).markWatchedAndSync, which prefers the realtime event and falls back togetOrCreate({ watch: true })when the event is dropped.waitForEvent.Test plan
lint-and-testpasses on this PRyarn workspace @stream-io/feeds-client run test stories.test.ts(requires Stream API credentials)Made with Cursor
Summary by CodeRabbit
Bug Fixes
Tests