-
-
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 all commits
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
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
39 changes: 39 additions & 0 deletions
39
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,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<void> { | ||
// 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; | ||
} |
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
198 changes: 198 additions & 0 deletions
198
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,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<void>(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(); | ||
}); | ||
}); | ||
}); |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to double check: We don't actually know if users handle this error, correct? If so, then
handled: false
is correct.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically the last point an unhandled error can reach on the server, so this should be fine 👍