diff --git a/app/entry.server.tsx b/app/entry.server.tsx index e63f4c39..c2c64404 100644 --- a/app/entry.server.tsx +++ b/app/entry.server.tsx @@ -13,6 +13,7 @@ import { import { getSessionRenewal, sessionKey } from './utils/auth.server.ts' import { init as initCron } from './utils/cron-runner.server.ts' import { getEnv, init as initEnv } from './utils/env.server.ts' +import { shouldReportToSentry } from './utils/error-reporting.server.ts' import { getInstanceInfo } from './utils/litefs.server.ts' import { NonceProvider } from './utils/nonce-provider.ts' import { authSessionStorage } from './utils/session.server.ts' @@ -136,6 +137,11 @@ export function handleError( if (request.signal.aborted) { return } + // RouteErrorResponses (405 missing action, OPTIONS, intentional 404s, etc.) + // are expected outcomes — same policy as GeneralErrorBoundary on the client. + if (!shouldReportToSentry(error)) { + return + } const requestContext = { url: request.url, method: request.method, diff --git a/app/routes/$.tsx b/app/routes/$.tsx index b8b718ee..d523818a 100644 --- a/app/routes/$.tsx +++ b/app/routes/$.tsx @@ -13,6 +13,13 @@ export async function loader() { throw new Response('Not found', { status: 404 }) } +// Scanners often POST to probe paths (/graphql, /api/gql, …). Without an +// action, React Router surfaces a 405 RouteErrorResponse; mirror the loader so +// unknown mutations get a normal 404 instead. +export async function action() { + throw new Response('Not found', { status: 404 }) +} + export default function NotFound() { // due to the loader, this component will never be rendered, but we'll return // the error boundary just in case. diff --git a/app/routes/resources+/theme-switch.tsx b/app/routes/resources+/theme-switch.tsx index d00c9983..52754137 100644 --- a/app/routes/resources+/theme-switch.tsx +++ b/app/routes/resources+/theme-switch.tsx @@ -3,6 +3,7 @@ import { parseWithZod } from '@conform-to/zod/v4' import { invariantResponse } from '@epic-web/invariant' import { data as json, + redirect, type ActionFunctionArgs, useFetcher, useFetchers, @@ -17,6 +18,12 @@ const ThemeFormSchema = z.object({ theme: z.enum(['system', 'light', 'dark']), }) +// Action-only resource route: a bare GET (bookmark, bot, refresh) has no +// loader otherwise and becomes a React Router RouteErrorResponse. +export function loader() { + return redirect('/') +} + export async function action({ request }: ActionFunctionArgs) { const formData = await request.formData() const submission = parseWithZod(formData, { diff --git a/app/utils/error-reporting.server.test.ts b/app/utils/error-reporting.server.test.ts new file mode 100644 index 00000000..bd73a532 --- /dev/null +++ b/app/utils/error-reporting.server.test.ts @@ -0,0 +1,99 @@ +import { expect, test } from 'bun:test' +import { isRouteErrorResponse } from 'react-router' +import { + isReactRouterMethodNoiseMessage, + shouldDropSentryEvent, + shouldReportToSentry, +} from './error-reporting.server.ts' + +test('reports unexpected Error instances', () => { + expect(shouldReportToSentry(new Error('boom'))).toBe(true) +}) + +test('reports unknown non-route values', () => { + expect(shouldReportToSentry('string failure')).toBe(true) + expect(shouldReportToSentry(null)).toBe(true) +}) + +test('does not report React Router route error responses', () => { + const missingAction = { + status: 405, + statusText: 'Method Not Allowed', + internal: true, + data: 'Error: You made a POST request to "/api/gql" but did not provide an `action` for route "routes/$", so there is no way to handle the request.', + error: new Error( + 'You made a POST request to "/api/gql" but did not provide an `action` for route "routes/$", so there is no way to handle the request.', + ), + } + const invalidOptions = { + status: 405, + statusText: 'Method Not Allowed', + internal: true, + data: 'Error: Invalid request method "OPTIONS"', + error: new Error('Invalid request method "OPTIONS"'), + } + const missingLoader = { + status: 400, + statusText: 'Bad Request', + internal: true, + data: 'Error: You made a GET request to "/resources/theme-switch" but did not provide a `loader` for route "routes/resources+/theme-switch", so there is no way to handle the request.', + error: new Error( + 'You made a GET request to "/resources/theme-switch" but did not provide a `loader` for route "routes/resources+/theme-switch", so there is no way to handle the request.', + ), + } + const intentionalNotFound = { + status: 404, + statusText: 'Not Found', + internal: false, + data: 'Not found', + } + + expect(isRouteErrorResponse(missingAction)).toBe(true) + expect(isRouteErrorResponse(invalidOptions)).toBe(true) + expect(isRouteErrorResponse(missingLoader)).toBe(true) + expect(isRouteErrorResponse(intentionalNotFound)).toBe(true) + + expect(shouldReportToSentry(missingAction)).toBe(false) + expect(shouldReportToSentry(invalidOptions)).toBe(false) + expect(shouldReportToSentry(missingLoader)).toBe(false) + expect(shouldReportToSentry(intentionalNotFound)).toBe(false) +}) + +test('matches React Router method-noise exception messages', () => { + expect( + isReactRouterMethodNoiseMessage('Invalid request method "OPTIONS"'), + ).toBe(true) + expect( + isReactRouterMethodNoiseMessage( + 'You made a POST request to "/api/gql" but did not provide an `action` for route "routes/$", so there is no way to handle the request.', + ), + ).toBe(true) + expect( + isReactRouterMethodNoiseMessage( + 'Error: You made a GET request to "/resources/theme-switch" but did not provide a `loader` for route "routes/resources+/theme-switch", so there is no way to handle the request.', + ), + ).toBe(true) + expect(isReactRouterMethodNoiseMessage('PrismaClientKnownRequestError')).toBe( + false, + ) + expect(isReactRouterMethodNoiseMessage('Unexpected server failure')).toBe( + false, + ) +}) + +test('drops Sentry events that only contain method-noise exceptions', () => { + expect( + shouldDropSentryEvent({ + exception: { + values: [{ type: 'Error', value: 'Invalid request method "OPTIONS"' }], + }, + }), + ).toBe(true) + expect( + shouldDropSentryEvent({ + exception: { + values: [{ type: 'Error', value: 'database connection refused' }], + }, + }), + ).toBe(false) +}) diff --git a/app/utils/error-reporting.server.ts b/app/utils/error-reporting.server.ts new file mode 100644 index 00000000..a5c003ff --- /dev/null +++ b/app/utils/error-reporting.server.ts @@ -0,0 +1,41 @@ +import { isRouteErrorResponse } from 'react-router' + +const reactRouterMethodNoise = + /^(Error:\s*)?(Invalid request method "|You made a (GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS) request to ")/i + +/** + * Whether an error caught by the server `handleError` hook should be sent to + * Sentry. + * + * React Router reports expected request outcomes (missing loader/action, + * unsupported methods like OPTIONS, intentional thrown Responses) as + * `RouteErrorResponse`s. Those are handled by route error boundaries and must + * not be treated as unexpected failures — matching + * `GeneralErrorBoundary` on the client. + */ +export function shouldReportToSentry(error: unknown): boolean { + return !isRouteErrorResponse(error) +} + +/** + * Narrow match for React Router's getInternalRouterError messages that bots and + * scanners trigger (unsupported method, missing loader/action). Used as a + * beforeSend safety net. + */ +export function isReactRouterMethodNoiseMessage(message: string): boolean { + return reactRouterMethodNoise.test(message) +} + +type SentryExceptionLike = { + exception?: { + values?: Array<{ type?: string | null; value?: string | null }> + } +} + +export function shouldDropSentryEvent(event: SentryExceptionLike): boolean { + const values = event.exception?.values ?? [] + return values.some((value) => { + const message = value.value ?? '' + return isReactRouterMethodNoiseMessage(message) + }) +} diff --git a/server/utils/monitoring.ts b/server/utils/monitoring.ts index 34a22651..877092ca 100644 --- a/server/utils/monitoring.ts +++ b/server/utils/monitoring.ts @@ -1,4 +1,5 @@ import * as Sentry from '@sentry/react-router' +import { shouldDropSentryEvent } from '#app/utils/error-reporting.server.ts' export function init() { Sentry.init({ @@ -23,6 +24,14 @@ export function init() { } return 1 }, + beforeSend(event) { + // Defense in depth for bot/scanner traffic that hits routes without a + // matching loader/action (grouped in Sentry as getInternalRouterError). + if (shouldDropSentryEvent(event)) { + return null + } + return event + }, beforeSendTransaction(event) { // ignore all healthcheck related transactions // note that name of header here is case-sensitive