feat(remix): Complete Remix 3 server error handling - #24878
Merged
chargome merged 2 commits intoOct 1, 2026
Conversation
Member
Author
|
bugbot run |
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
September 30, 2026 10:29
6636642 to
d5cfe2f
Compare
Contributor
size-limit report 📦
|
chargome
added this pull request to stack #24884
September 30, 2026 11:41
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
September 30, 2026 13:41
d5cfe2f to
57ee22d
Compare
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
3 times, most recently
from
September 30, 2026 14:43
513745c to
9fa7790
Compare
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
September 30, 2026 15:01
9fa7790 to
7083500
Compare
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
October 1, 2026 08:12
7083500 to
c199a22
Compare
Member
Author
|
bugbot run |
1 similar comment
Member
Author
|
bugbot run |
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
October 1, 2026 08:21
c199a22 to
3acd8b2
Compare
Member
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
October 1, 2026 08:32
3acd8b2 to
efcec61
Compare
Member
Author
|
bugbot run |
chargome
marked this pull request as ready for review
October 1, 2026 08:38
chargome
requested review from
nicohrubec and
s1gr1d
and removed request for
a team
October 1, 2026 08:38
chargome
requested review from
isaacs and
mydea
and removed request for
a team
October 1, 2026 08:38
Covers the error paths the router middleware alone does not see, and stops reporting things that are not faults. `createRequestListener` in `@remix-run/node-fetch-server` is patched to install an `onError` that chains to the app's own. This is the only hook that covers an app whose fetch handler is not a router. It needs no abort guard: the listener checks `isRequestAbortError` itself and returns before calling `onError`. The middleware captures too, so an event carries the route and request already on the scope. That path does need the abort guard, because `raceRequestAbort` rejects with `signal.reason` when the client disconnects. Without it every user navigating away mid request creates an issue. Removing the guard makes the new e2e test fail. `shouldHandleError` follows `@sentry/hono`: skip 3xx and 4xx errors carrying a numeric `status`, capture the rest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chargome
force-pushed
the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
from
October 1, 2026 13:33
efcec61 to
5ad3a52
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+38
to
42
| captureRequestError(error, context.request, 'auto.http.remix_v3.middleware'); | ||
| throw error; | ||
| } | ||
|
|
||
| setResponseStatus(response); |
Contributor
There was a problem hiding this comment.
Bug: Primitive values thrown from Remix route handlers are reported twice because the de-duplication logic only works for Error objects, causing both the middleware and onError handler to capture the error.
Severity: LOW
Suggested Fix
The error handling logic should be updated to prevent double reporting for all thrown types. One possible solution is to wrap any captured primitive in an Error object at the earliest capture point. This would allow the existing de-duplication mechanism, which marks objects to prevent re-capture, to function correctly for all thrown values and ensure they are only reported to Sentry once.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/remix/src/v3/server/middleware.ts#L38-L42
Potential issue: Primitive values, such as strings or numbers, thrown from Remix route
handlers can be captured and reported to Sentry twice. This occurs because the
de-duplication mechanism, which relies on marking an error as caught, does not work for
primitives. As a result, a thrown primitive is first captured by the router middleware
and then again by the `onError` handler. This leads to two separate Sentry events for
the same underlying error, creating noise and potentially confusing error tracking data.
While this is an acknowledged limitation in a code comment, it remains an issue that
will affect applications that throw primitives in their handlers.
Also affects:
packages/remix/src/v3/server/instrument.ts:96~100
Did we get this right? 👍 / 👎 to inform future reviews.
s1gr1d
approved these changes
Oct 1, 2026
Comment on lines
+92
to
+97
| options.onError = error => { | ||
| captureRequestError(error, undefined, 'auto.http.remix_v3.on_error'); | ||
|
|
||
| // Chained so the app keeps its own response. | ||
| return appOnError?.(error); | ||
| }; |
Member
There was a problem hiding this comment.
When overwriting the error handler, we would remove their default error handling, no?
Maybe we can test that that the error is still logged to the console (like in their default error handler).
Setting `onError` replaces the listener's default handler, which logs the error to the console. When the app passed no handler of its own, the hook now logs the same way after capturing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
|
bugbot run |
chargome
deleted the
charlygomez/js-3767-complete-remix-3-server-error-handling
branch
October 1, 2026 15:09
Member
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ad5bcdb. Configure here.
47 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Covers the error paths the router middleware alone does not see, and stops reporting things that are not faults.
createRequestListenerin@remix-run/node-fetch-serveris patched to install anonErrorthat chains to the app's own. This is the only hook that covers an app whose fetch handler is not a router. It needs no abort guard: the listener checksisRequestAbortErroritself and returns before callingonError.The middleware captures too, so an event carries the route and request already on the scope. That path does need the abort guard, because
raceRequestAbortrejects withsignal.reasonwhen the client disconnects. Without it every user navigating away mid request creates an issue. Removing the guard makes the new e2e test fail.shouldHandleErrorfollows@sentry/hono: skip 3xx and 4xx errors carrying a numericstatus, capture the rest.Fixes #24664