handleError inconsistently returns wrapped & unwrapped ErrorResponse objects, making it hard to filter errors from telemetry services

[huw]

What version of Remix are you using?

v1.19.3

Are all your remix dependencies & dev-dependencies using the same version?

• Yes

Diagnosis

I started running into this because I upgraded my Sentry integration to use the v2 future flags according to their guide, and found I was still getting a handle of 4xx errors reported to Sentry despite filtering them out like so:

export const handleError = async (
  error: unknown,
  { request, context: { sentry } }: DataFunctionArgs & { context: { sentry: Hub } }
) => {
  if (error instanceof Error) {
    await captureRemixServerException(sentry, error, "remix.server", request);
  } else if (!(isRouteErrorResponse(error) && error.status < 500)) {
    // Ignore 4xx errors since they're frequent and usually relate to user error.
    sentry.captureException(error);
  }
};

Specifically, I was getting You made a POST request to "/" but did not provide an action for route "routes/_index", so there is no way to handle the request., normally a 405 ErrorResponse, thrown as an Error object whenever an automated crawler tried to POST to my website. However, 404s would be thrown as ordinary ErrorResponse objects so I could filter them using the above code.

I isolated the issue to:

remix/packages/remix-server-runtime/server.ts

Lines 186 to 188 in 8779b24

	if (error.error) {
	handleError(error.error);
	}

Here, whenever the server runtime detects an ErrorResponse, it tries to unwrap the original error and send it to the user, if it exists. This is understandable behaviour because sometimes the user might want to manually attach an error to their response, especially if it’s a plain old 500 without any other useful information.

However:

https://github.com/remix-run/react-router/blob/496b1fe8253643171ecca6e6a945d98386c4eb00/packages/router/router.ts#L4142-L4147

react-router creates a good number of ErrorResponses by passing a dummy new Error(errorMessage) to the object, which then gets unwrapped. This gives me less information when looking at handleError, and I can’t easily filter out 4xx status codes. Instead, I have to manually compare the error message with hard-coded strings, which is really brittle. (I assume the reason I don’t see this for 404s if because it creates those ErrorResponses in a different way).

Proposed Solutions

Before I write up a PR, I want to clarify with the Remix team what the best solution would be:

1. Don’t unwrap error.error, and instead re-export isRouteErrorResponse from @remix-run/server-runtime and downstream dependencies. This would allow the user to perform similar checks in handleError that they might do on the frontend.
2. Alternatively, omit the dummy error from the ErrorResponse in the react-router code.

(More broadly, the story for error telemetry in v2 is painful. Those Sentry docs illustrate the problem—we don’t clearly demarcate between ‘client-side’ and ‘server-side’ errors in the ErrorBoundary (noting that even those terms can be misnomers in the context of RR), so the user has to add their own filters to prevent duplication.)

[brophdawg11]

Thank you for the detailed explanation! This seems to be an oversight - document requests sent the full ErrorResponse through, but _data/resource requests did not. This is fixed by #7211 and will be available in Remix v2

[huw]

Legend! Love ya ❤️