fix(hooks): propagate errors in useAsyncCallback to the React error boundary#612
fix(hooks): propagate errors in useAsyncCallback to the React error boundary#612Just-Insane wants to merge 5 commits intoSableClient:devfrom
Conversation
Wrap the inner callback with a no-op .catch() so fire-and-forget call sites (e.g. loadSrc in useEffect) do not produce 'Uncaught (in promise)' console warnings. The promise is still returned and re-thrown for callers that await or chain, so intentional error handling is unaffected.
Just-Insane
force-pushed
the
fix/async-callback-rejections
branch
from
24fd8f7 to
918f2d7
Compare
There was a problem hiding this comment.
Pull request overview
Fixes error-handling behavior in the useAsyncCallback hook so callers that await/chain the returned promise can observe rejections (instead of needing to defensively swallow them), with accompanying test and changeset updates.
Changes:
- Wrap
useAsyncCallback’s returned function to attach a no-op rejection handler while still returning the original rejecting promise. - Update the hook test to assert the promise rejects when the async callback throws.
- Add a changeset describing the intended behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/app/hooks/useAsyncCallback.ts |
Adds a wrapper around the async callback promise to attach a no-op .catch while returning the original promise. |
src/app/hooks/useAsyncCallback.test.tsx |
Updates the error-path test to assert the returned promise rejects with the thrown error. |
.changeset/async-callback-rejections.md |
Adds a patch changeset entry describing the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Re-throw preserves rejection for callers that await/chain; the no-op .catch | ||
| // suppresses "Uncaught (in promise)" for fire-and-forget call sites (e.g. | ||
| // loadSrc() in a useEffect) without swallowing the error from intentional callers. | ||
| const callback = useCallback( | ||
| (...args: TArgs): Promise<TData> => { | ||
| const p = innerCallback(...args); | ||
| p.catch(() => {}); | ||
| return p; | ||
| }, | ||
| [innerCallback] | ||
| ) as AsyncCallback<TArgs, TData>; | ||
|
|
There was a problem hiding this comment.
The added no-op p.catch(() => {}) marks the rejection as handled, which prevents unhandledrejection from firing. In this repo Sentry is initialized with default browser integrations, so this will likely stop fire-and-forget failures from being captured/reported and can make production errors harder to detect. If the goal is to surface failures to the app’s error handling, consider either (a) removing the no-op catch and fixing call sites that truly need to ignore errors, or (b) reporting the error explicitly (e.g., via Sentry/logging) and/or rethrowing on a separate task so it still reaches global error handling.
| // Re-throw preserves rejection for callers that await/chain; the no-op .catch | |
| // suppresses "Uncaught (in promise)" for fire-and-forget call sites (e.g. | |
| // loadSrc() in a useEffect) without swallowing the error from intentional callers. | |
| const callback = useCallback( | |
| (...args: TArgs): Promise<TData> => { | |
| const p = innerCallback(...args); | |
| p.catch(() => {}); | |
| return p; | |
| }, | |
| [innerCallback] | |
| ) as AsyncCallback<TArgs, TData>; | |
| const callback = innerCallback as AsyncCallback<TArgs, TData>; |
| default: patch | ||
| --- | ||
|
|
||
| Fix unhandled promise rejections in useAsyncCallback by propagating errors to the error boundary |
There was a problem hiding this comment.
This changeset entry claims errors are propagated “to the error boundary”, but the current implementation adds a no-op .catch that suppresses unhandled promise rejections (and React error boundaries don’t catch async rejections anyway). Please align the changeset wording with the actual behavior, or adjust the implementation to truly route unhandled async errors into the app’s chosen error-boundary/reporting mechanism.
…behaviour Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
useAsyncCallbackwas silently swallowing unhandled rejections from async callbacks. React's error boundary never received them, so errors were invisible to users and the UI could be left in a stuck loading state indefinitely. This fix wraps internal invocations so unhandled rejections are re-thrown on the next tick and surface to the nearest React error boundary.Fixes #
Type of change
Checklist:
AI disclosure:
The error propagation wrapper in
useAsyncCallbackwas partially AI assisted. I reviewed the boundary behaviour and verified errors surface correctly rather than being swallowed.