diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/app/entry.server.tsx index 97260755da21..738cd1515a4d 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/app/entry.server.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/app/entry.server.tsx @@ -15,12 +15,4 @@ const handleRequest = Sentry.createSentryHandleRequest({ export default handleRequest; -export const handleError: HandleErrorFunction = (error, { request }) => { - // React Router may abort some interrupted requests, don't log those - if (!request.signal.aborted) { - Sentry.captureException(error); - - // make sure to still log the error so you can see it - console.error(error); - } -}; +export const handleError: HandleErrorFunction = Sentry.createSentryHandleError({ logErrors: true }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/errors/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/errors/errors.server.test.ts index d702f8cee597..2759bfecb67e 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/errors/errors.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-custom/tests/errors/errors.server.test.ts @@ -20,7 +20,8 @@ test.describe('server-side errors', () => { type: 'Error', value: errorMessage, mechanism: { - handled: true, + handled: false, + type: 'react-router', }, }, ], @@ -67,7 +68,8 @@ test.describe('server-side errors', () => { type: 'Error', value: errorMessage, mechanism: { - handled: true, + handled: false, + type: 'react-router', }, }, ], diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/app/entry.server.tsx index 97260755da21..738cd1515a4d 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/app/entry.server.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/app/entry.server.tsx @@ -15,12 +15,4 @@ const handleRequest = Sentry.createSentryHandleRequest({ export default handleRequest; -export const handleError: HandleErrorFunction = (error, { request }) => { - // React Router may abort some interrupted requests, don't log those - if (!request.signal.aborted) { - Sentry.captureException(error); - - // make sure to still log the error so you can see it - console.error(error); - } -}; +export const handleError: HandleErrorFunction = Sentry.createSentryHandleError({ logErrors: true }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/errors/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/errors/errors.server.test.ts index d702f8cee597..2759bfecb67e 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/errors/errors.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-node-20-18/tests/errors/errors.server.test.ts @@ -20,7 +20,8 @@ test.describe('server-side errors', () => { type: 'Error', value: errorMessage, mechanism: { - handled: true, + handled: false, + type: 'react-router', }, }, ], @@ -67,7 +68,8 @@ test.describe('server-side errors', () => { type: 'Error', value: errorMessage, mechanism: { - handled: true, + handled: false, + type: 'react-router', }, }, ], diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx index 97260755da21..738cd1515a4d 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx @@ -15,12 +15,4 @@ const handleRequest = Sentry.createSentryHandleRequest({ export default handleRequest; -export const handleError: HandleErrorFunction = (error, { request }) => { - // React Router may abort some interrupted requests, don't log those - if (!request.signal.aborted) { - Sentry.captureException(error); - - // make sure to still log the error so you can see it - console.error(error); - } -}; +export const handleError: HandleErrorFunction = Sentry.createSentryHandleError({ logErrors: true }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/errors/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/errors/errors.server.test.ts index d702f8cee597..2759bfecb67e 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/errors/errors.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/errors/errors.server.test.ts @@ -20,7 +20,8 @@ test.describe('server-side errors', () => { type: 'Error', value: errorMessage, mechanism: { - handled: true, + handled: false, + type: 'react-router', }, }, ], @@ -67,7 +68,8 @@ test.describe('server-side errors', () => { type: 'Error', value: errorMessage, mechanism: { - handled: true, + handled: false, + type: 'react-router', }, }, ], diff --git a/packages/react-router/src/server/createSentryHandleError.ts b/packages/react-router/src/server/createSentryHandleError.ts new file mode 100644 index 000000000000..65114d035655 --- /dev/null +++ b/packages/react-router/src/server/createSentryHandleError.ts @@ -0,0 +1,39 @@ +import { captureException, flushIfServerless } from '@sentry/core'; +import type { ActionFunctionArgs, HandleErrorFunction, LoaderFunctionArgs } from 'react-router'; + +export type SentryHandleErrorOptions = { + logErrors?: boolean; +}; + +/** + * A complete Sentry-instrumented handleError implementation that handles error reporting + * + * @returns A Sentry-instrumented handleError function + */ +export function createSentryHandleError({ logErrors = false }: SentryHandleErrorOptions): HandleErrorFunction { + const handleError = async function handleError( + error: unknown, + args: LoaderFunctionArgs | ActionFunctionArgs, + ): Promise { + // React Router may abort some interrupted requests, don't report those + if (!args.request.signal.aborted) { + captureException(error, { + mechanism: { + type: 'react-router', + handled: false, + }, + }); + if (logErrors) { + // eslint-disable-next-line no-console + console.error(error); + } + try { + await flushIfServerless(); + } catch { + // Ignore flush errors to ensure error handling completes gracefully + } + } + }; + + return handleError; +} diff --git a/packages/react-router/src/server/index.ts b/packages/react-router/src/server/index.ts index b42b769a78e6..91520059d3fb 100644 --- a/packages/react-router/src/server/index.ts +++ b/packages/react-router/src/server/index.ts @@ -6,3 +6,4 @@ export { wrapSentryHandleRequest, sentryHandleRequest, getMetaTagTransformer } f export { createSentryHandleRequest, type SentryHandleRequestOptions } from './createSentryHandleRequest'; export { wrapServerAction } from './wrapServerAction'; export { wrapServerLoader } from './wrapServerLoader'; +export { createSentryHandleError, type SentryHandleErrorOptions } from './createSentryHandleError'; diff --git a/packages/react-router/test/server/createSentryHandleError.test.ts b/packages/react-router/test/server/createSentryHandleError.test.ts new file mode 100644 index 000000000000..0cad5d2a04ba --- /dev/null +++ b/packages/react-router/test/server/createSentryHandleError.test.ts @@ -0,0 +1,198 @@ +import * as core from '@sentry/core'; +import type { ActionFunctionArgs, LoaderFunctionArgs } from 'react-router'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createSentryHandleError } from '../../src/server/createSentryHandleError'; + +vi.mock('@sentry/core', () => ({ + captureException: vi.fn(), + flushIfServerless: vi.fn().mockResolvedValue(undefined), +})); + +const mechanism = { + handled: false, + type: 'react-router', +}; + +describe('createSentryHandleError', () => { + const mockCaptureException = vi.mocked(core.captureException); + const mockFlushIfServerless = vi.mocked(core.flushIfServerless); + const mockConsoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const mockError = new Error('Test error'); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + mockConsoleError.mockClear(); + }); + + const createMockArgs = (aborted: boolean): LoaderFunctionArgs => { + const controller = new AbortController(); + if (aborted) { + controller.abort(); + } + + const request = { + signal: controller.signal, + } as Request; + + return { request } as LoaderFunctionArgs; + }; + + describe('with default options', () => { + it('should create a handle error function with logErrors disabled by default', async () => { + const handleError = createSentryHandleError({}); + + expect(typeof handleError).toBe('function'); + }); + + it('should capture exception and flush when request is not aborted', async () => { + const handleError = createSentryHandleError({}); + const mockArgs = createMockArgs(false); + + await handleError(mockError, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + expect(mockConsoleError).not.toHaveBeenCalled(); + }); + + it('should not capture exception when request is aborted', async () => { + const handleError = createSentryHandleError({}); + const mockArgs = createMockArgs(true); + + await handleError(mockError, mockArgs); + + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(mockFlushIfServerless).not.toHaveBeenCalled(); + expect(mockConsoleError).not.toHaveBeenCalled(); + }); + }); + + describe('with logErrors enabled', () => { + it('should log errors to console when logErrors is true', async () => { + const handleError = createSentryHandleError({ logErrors: true }); + const mockArgs = createMockArgs(false); + + await handleError(mockError, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + expect(mockConsoleError).toHaveBeenCalledWith(mockError); + }); + + it('should not log errors to console when request is aborted even with logErrors enabled', async () => { + const handleError = createSentryHandleError({ logErrors: true }); + const mockArgs = createMockArgs(true); + + await handleError(mockError, mockArgs); + + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(mockFlushIfServerless).not.toHaveBeenCalled(); + expect(mockConsoleError).not.toHaveBeenCalled(); + }); + }); + + describe('with logErrors disabled explicitly', () => { + it('should not log errors to console when logErrors is false', async () => { + const handleError = createSentryHandleError({ logErrors: false }); + const mockArgs = createMockArgs(false); + + await handleError(mockError, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + expect(mockConsoleError).not.toHaveBeenCalled(); + }); + }); + + describe('with different error types', () => { + it('should handle string errors', async () => { + const handleError = createSentryHandleError({}); + const stringError = 'String error message'; + const mockArgs = createMockArgs(false); + + await handleError(stringError, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(stringError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + }); + + it('should handle null/undefined errors', async () => { + const handleError = createSentryHandleError({}); + const mockArgs = createMockArgs(false); + + await handleError(null, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(null, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + }); + + it('should handle custom error objects', async () => { + const handleError = createSentryHandleError({}); + const customError = { message: 'Custom error', code: 500 }; + const mockArgs = createMockArgs(false); + + await handleError(customError, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(customError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + }); + }); + + describe('with ActionFunctionArgs', () => { + it('should work with ActionFunctionArgs instead of LoaderFunctionArgs', async () => { + const handleError = createSentryHandleError({ logErrors: true }); + const mockArgs = createMockArgs(false) as ActionFunctionArgs; + + await handleError(mockError, mockArgs); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + expect(mockConsoleError).toHaveBeenCalledWith(mockError); + }); + }); + + describe('flushIfServerless behavior', () => { + it('should wait for flushIfServerless to complete', async () => { + const handleError = createSentryHandleError({}); + + let resolveFlush: () => void; + const flushPromise = new Promise(resolve => { + resolveFlush = resolve; + }); + + mockFlushIfServerless.mockReturnValueOnce(flushPromise); + + const mockArgs = createMockArgs(false); + + const startTime = Date.now(); + + const handleErrorPromise = handleError(mockError, mockArgs); + + setTimeout(() => resolveFlush(), 10); + + await handleErrorPromise; + const endTime = Date.now(); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + expect(endTime - startTime).toBeGreaterThanOrEqual(10); + }); + + it('should handle flushIfServerless rejection gracefully', async () => { + const handleError = createSentryHandleError({}); + + mockFlushIfServerless.mockRejectedValueOnce(new Error('Flush failed')); + + const mockArgs = createMockArgs(false); + + await expect(handleError(mockError, mockArgs)).resolves.toBeUndefined(); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError, { mechanism }); + expect(mockFlushIfServerless).toHaveBeenCalled(); + }); + }); +});