-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(react-router): Add createSentryHandleError
#17235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
34 changes: 34 additions & 0 deletions
34
packages/react-router/src/server/createSentryHandleError.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
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<void> { | ||
// React Router may abort some interrupted requests, don't report those | ||
if (!args.request.signal.aborted) { | ||
captureException(error); | ||
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; | ||
} |
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
200 changes: 200 additions & 0 deletions
200
packages/react-router/test/server/createSentryHandleError.test.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,200 @@ | ||
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), | ||
})); | ||
|
||
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(); | ||
}); | ||
|
||
// Helper function to create mock args with proper Request structure | ||
const createMockArgs = (aborted: boolean): LoaderFunctionArgs => { | ||
const controller = new AbortController(); | ||
if (aborted) { | ||
controller.abort(); | ||
} | ||
|
||
const request = new Request('http://test.com', { | ||
signal: controller.signal, | ||
}); | ||
|
||
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); | ||
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); | ||
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); | ||
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); | ||
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); | ||
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); | ||
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); | ||
expect(mockFlushIfServerless).toHaveBeenCalled(); | ||
expect(mockConsoleError).toHaveBeenCalledWith(mockError); | ||
}); | ||
}); | ||
|
||
describe('flushIfServerless behavior', () => { | ||
it('should wait for flushIfServerless to complete', async () => { | ||
const handleError = createSentryHandleError({}); | ||
|
||
// Create a promise that resolves after 10ms | ||
let resolveFlush: () => void; | ||
const flushPromise = new Promise<void>(resolve => { | ||
resolveFlush = resolve; | ||
}); | ||
|
||
// Mock flushIfServerless to return our controlled promise | ||
mockFlushIfServerless.mockReturnValueOnce(flushPromise); | ||
|
||
const mockArgs = createMockArgs(false); | ||
|
||
const startTime = Date.now(); | ||
|
||
// Start the handleError call | ||
const handleErrorPromise = handleError(mockError, mockArgs); | ||
|
||
// Resolve the flush after 10ms | ||
setTimeout(() => resolveFlush(), 10); | ||
|
||
await handleErrorPromise; | ||
const endTime = Date.now(); | ||
|
||
expect(mockCaptureException).toHaveBeenCalledWith(mockError); | ||
expect(mockFlushIfServerless).toHaveBeenCalled(); | ||
expect(endTime - startTime).toBeGreaterThanOrEqual(10); | ||
}); | ||
|
||
it('should handle flushIfServerless rejection gracefully', async () => { | ||
const handleError = createSentryHandleError({}); | ||
|
||
// Make flushIfServerless reject | ||
mockFlushIfServerless.mockRejectedValueOnce(new Error('Flush failed')); | ||
|
||
const mockArgs = createMockArgs(false); | ||
|
||
// This should not throw | ||
await expect(handleError(mockError, mockArgs)).resolves.toBeUndefined(); | ||
|
||
expect(mockCaptureException).toHaveBeenCalledWith(mockError); | ||
expect(mockFlushIfServerless).toHaveBeenCalled(); | ||
}); | ||
}); | ||
}); |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.