fix: rename UnhandledErrorType.FetchingOwnCapabilitiesOnNewActivity t…#229
fix: rename UnhandledErrorType.FetchingOwnCapabilitiesOnNewActivity t…#229
Conversation
…o UnhandledErrorType.FetchingOwnFieldsOnNewActivity
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a retry utility and integrates it into feed synchronization and ownBatch calls; renames an unhandled error enum member; adds/updates retry-focused unit and integration tests and logging; modifies reconnection/reconciliation dispatch and a connection test timeout; and subscribes to unhandled errors in the sample React app. Changes
Sequence Diagram(s)sequenceDiagram
participant Feed as Feed.synchronize
participant Retry as withRetry
participant HTTP as ownBatch / getOrCreate
participant API as Stream API
Feed->>Retry: provide request function
Retry->>HTTP: invoke request (attempt 1)
HTTP->>API: HTTP request
API-->>HTTP: 5xx / network error
HTTP-->>Retry: reject error
alt isRetryable / shouldRetry true
Retry-->>Retry: wait backoff
Retry->>HTTP: invoke request (attempt N)
HTTP->>API: HTTP request
API-->>HTTP: success
HTTP-->>Retry: resolve response
Retry-->>Feed: return response
else non-retryable (4xx) or max attempts reached
Retry-->>Feed: throw error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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
🤖 Fix all issues with AI agents
In `@sample-apps/react-demo/app/ClientApp.tsx`:
- Around line 77-85: The effect listening to client.on('errors.unhandled') uses
CURRENT_USER but does not include it in the dependency array, causing a
react-hooks/exhaustive-deps lint error; update the useEffect that registers the
'errors.unhandled' handler (the effect referencing client?.on and calling
client?.disconnectUser() and client?.connectUser(CURRENT_USER,
CURRENT_USER.token)) to include CURRENT_USER in the dependency array, and while
there rename the callback parameter from "_" to a descriptive name like "error"
to clarify its purpose in the console.error call.
| useEffect(() => { | ||
| const off = client?.on('errors.unhandled', async (_) => { | ||
| console.error('errors.unhandled', _); | ||
| await client?.disconnectUser(); | ||
| await client?.connectUser(CURRENT_USER, CURRENT_USER.token); | ||
| }); | ||
|
|
||
| return () => off?.(); | ||
| }, [client]); |
There was a problem hiding this comment.
Add missing CURRENT_USER dependency to fix ESLint error and unblock the pipeline.
CURRENT_USER is referenced inside the effect callback but is not listed in the dependency array, causing the react-hooks/exhaustive-deps rule violation. Since CURRENT_USER is already memoized, adding it to the dependency array is safe and won't cause unnecessary effect re-runs.
Additionally, consider renaming _ to error since it's actually used in console.error.
🔧 Proposed fix
useEffect(() => {
- const off = client?.on('errors.unhandled', async (_) => {
- console.error('errors.unhandled', _);
+ const off = client?.on('errors.unhandled', async (error) => {
+ console.error('errors.unhandled', error);
await client?.disconnectUser();
await client?.connectUser(CURRENT_USER, CURRENT_USER.token);
});
return () => off?.();
-}, [client]);
+}, [client, CURRENT_USER]);🧰 Tools
🪛 ESLint
[error] 85-85: React Hook useEffect has a missing dependency: 'CURRENT_USER'. Either include it or remove the dependency array.
(react-hooks/exhaustive-deps)
🪛 GitHub Actions: Lint and test
[error] 85-85: React Hook useEffect has a missing dependency: 'CURRENT_USER'. Either include it or remove the dependency array. (react-hooks/exhaustive-deps)
🪛 GitHub Check: lint-and-test (22.x)
[failure] 85-85:
React Hook useEffect has a missing dependency: 'CURRENT_USER'. Either include it or remove the dependency array
🤖 Prompt for AI Agents
In `@sample-apps/react-demo/app/ClientApp.tsx` around lines 77 - 85, The effect
listening to client.on('errors.unhandled') uses CURRENT_USER but does not
include it in the dependency array, causing a react-hooks/exhaustive-deps lint
error; update the useEffect that registers the 'errors.unhandled' handler (the
effect referencing client?.on and calling client?.disconnectUser() and
client?.connectUser(CURRENT_USER, CURRENT_USER.token)) to include CURRENT_USER
in the dependency array, and while there rename the callback parameter from "_"
to a descriptive name like "error" to clarify its purpose in the console.error
call.
1 participant
…o UnhandledErrorType.FetchingOwnFieldsOnNewActivity
🎫 Ticket: a follow-up for https://linear.app/stream/issue/REACT-685/backfill-all-own-fields-from-ownbatch-request
📑 Docs: https://github.com/GetStream/docs-content/pull/990
💡 Overview
It renames the unhandled error type for failing to fetch own_capabilities, it's fine to rename this error, because it's not yet documented anywhere.
This PR also adds an automatic retry to state sync and ownCapabilities fetching, ensuring that unhandled errors are only thrown when necessary.
📝 Implementation notes
Summary by CodeRabbit
New Features
Reliability Improvements
Tests
Note: No direct UI changes; primarily stability and reliability updates.