Sentry Remix integration reports thrown redirects as errors

[justinwaite]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/remix

SDK Version

7.88.0

Framework Version

Remix 2.4.0

Link to Sentry event

https://mining-syndicate.sentry.io/issues/4725655099/?environment=development&project=6347078&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=30d&stream_index=0&utc=true

SDK Setup

entry.server.ts

Sentry.init({
  dsn: SENTRY_DSN,
  environment: SENTRY_ENV,
  tracesSampler(ctx) {
    if (ctx.request?.url?.includes('healthcheck') || ctx.location?.pathname.includes('healthcheck')) {
      return 0;
    }

    return SENTRY_ENV === 'production' ? 0.1 : 1;
  },
});

entry.client.tsx

Sentry.init({
  dsn: window.ENV.SENTRY_DSN,
  environment: window.ENV.SENTRY_ENV,
  tracesSampleRate: window.ENV.SENTRY_ENV === 'production' ? 0.1 : 1,
  replaysSessionSampleRate: 0.1,
  replaysOnErrorSampleRate: 1,

  integrations: [
    new Sentry.BrowserTracing({
      routingInstrumentation: Sentry.remixRouterInstrumentation(React.useEffect, useLocation, useMatches),
    }),
    new Sentry.Replay(),
  ],
});

server.js (custom express server)

const sentryCreateRequestHandler = wrapExpressCreateRequestHandler(createRequestHandler);
...
const remixHandler =
  MODE === 'development'
    ? await createDevRequestHandler(initialBuild)
    : sentryCreateRequestHandler({
        build: initialBuild,
        mode: initialBuild.mode,
        getLoadContext,
      });

Steps to Reproduce

In my loaders I have a function that checks if the user exists. If the user does not exist, I throw a redirect response.

e.g.

export function loader({request) {
  if (!getUser(request)) {
    throw new redirect('/login');
  }
}

Expected Result

The thrown redirect should be ignored by Sentry instead of logged as an error

Actual Result

The redirects are being flagged as errors.

I believe this could have been introduced by these lines in the following PR: 69b308d#diff-754e32c1c14ac3c3c93eedeb7680548b986bc02a8b0bc63d2efc189210322acdR84-R88

If you notice, it used to check if the error is a response object, however there was logic added to check if we are in Remix V2, and to only check if the error is a RouteErrorResponse (which is different).

Perhaps the logic here should be (notice removal of the else)

  if (IS_REMIX_V2 && isRouteErrorResponse(err) && err.status < 500) {
    return;
  }
  
  if (isResponse(err) && err.status < 500) {
    return;
  }

[justinwaite]

This could also be the culprit: https://github.com/getsentry/sentry-javascript/tree/develop/packages/remix/src/utils#L255

Perhaps this should be negated?

Current:

      if (isRemixV2 ? isResponse(err) : true) {
        await captureRemixServerException(err, name, args.request);
      }

Perhaps corrected?

      if (isRemixV2 ? !isResponse(err) : true) {
        await captureRemixServerException(err, name, args.request);
      }

[Lms24]

Hi, thanks for reporting! We shouldn't capture redirects.

@onurtemizkan please take a look at this, thanks!