Skip to content
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

feat(react): Expose eventId on React ErrorBoundary #2704

Merged
merged 1 commit into from Jun 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
- [tracing] fix: Add manual Location typing (#2700)
- [react] feat: Expose eventId on ErrorBoundary component

## 5.18.1

Expand Down
24 changes: 14 additions & 10 deletions packages/react/src/errorboundary.tsx
Expand Up @@ -8,6 +8,7 @@ export type FallbackRender = (fallback: {
error: Error | null;
componentStack: string | null;
resetError(): void;
eventId: string | null;
}) => React.ReactNode;

export type ErrorBoundaryProps = {
Expand All @@ -30,23 +31,25 @@ export type ErrorBoundaryProps = {
fallback?: React.ReactNode | FallbackRender;
// tslint:enable no-null-undefined-union
/** Called with the error boundary encounters an error */
onError?(error: Error, componentStack: string): void;
onError?(error: Error, componentStack: string, eventId: string): void;
/** Called on componentDidMount() */
onMount?(): void;
/** Called if resetError() is called from the fallback render props function */
onReset?(error: Error | null, componentStack: string | null): void;
onReset?(error: Error | null, componentStack: string | null, eventId: string | null): void;
/** Called on componentWillUnmount() */
onUnmount?(error: Error | null, componentStack: string | null): void;
onUnmount?(error: Error | null, componentStack: string | null, eventId: string | null): void;
};

type ErrorBoundaryState = {
componentStack: string | null;
error: Error | null;
eventId: string | null;
};

const INITIAL_STATE = {
componentStack: null,
error: null,
eventId: null,
};

/**
Expand All @@ -60,15 +63,15 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
const eventId = Sentry.captureException(error, { contexts: { react: { componentStack } } });
const { onError, showDialog, dialogOptions } = this.props;
if (onError) {
onError(error, componentStack);
onError(error, componentStack, eventId);
}
if (showDialog) {
Sentry.showReportDialog({ ...dialogOptions, eventId });
}

// componentDidCatch is used over getDerivedStateFromError
// so that componentStack is accessible through state.
this.setState({ error, componentStack });
this.setState({ error, componentStack, eventId });
}

public componentDidMount(): void {
Expand All @@ -79,31 +82,32 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
}

public componentWillUnmount(): void {
const { error, componentStack } = this.state;
const { error, componentStack, eventId } = this.state;
const { onUnmount } = this.props;
if (onUnmount) {
onUnmount(error, componentStack);
onUnmount(error, componentStack, eventId);
}
}

public resetErrorBoundary = () => {
const { onReset } = this.props;
const { error, componentStack, eventId } = this.state;
if (onReset) {
onReset(this.state.error, this.state.componentStack);
onReset(error, componentStack, eventId);
}
this.setState(INITIAL_STATE);
};

public render(): React.ReactNode {
const { fallback } = this.props;
const { error, componentStack } = this.state;
const { error, componentStack, eventId } = this.state;

if (error) {
if (React.isValidElement(fallback)) {
return fallback;
}
if (typeof fallback === 'function') {
return fallback({ error, componentStack, resetError: this.resetErrorBoundary }) as FallbackRender;
return fallback({ error, componentStack, resetError: this.resetErrorBoundary, eventId }) as FallbackRender;
}

// Fail gracefully if no fallback provided
Expand Down
17 changes: 10 additions & 7 deletions packages/react/test/errorboundary.test.tsx
Expand Up @@ -22,10 +22,10 @@ const TestApp: React.FC<ErrorBoundaryProps> = ({ children, ...props }) => {
return (
<ErrorBoundary
{...props}
onReset={(err: Error, stack: string) => {
onReset={(...args) => {
setError(false);
if (props.onReset) {
props.onReset(err, stack);
props.onReset(...args);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure we pass all the arguments, instead of having to edit this all the time when onReset changes.

}
}}
>
Expand Down Expand Up @@ -107,7 +107,7 @@ describe('ErrorBoundary', () => {
expect(mockOnUnmount).toHaveBeenCalledTimes(0);
unmount();
expect(mockOnUnmount).toHaveBeenCalledTimes(1);
expect(mockOnUnmount).toHaveBeenCalledWith(null, null);
expect(mockOnUnmount).toHaveBeenCalledWith(null, null, null);
});

it('renders children correctly when there is no error', () => {
Expand Down Expand Up @@ -140,12 +140,14 @@ describe('ErrorBoundary', () => {
it('renders a render props component', async () => {
let errorString = '';
let compStack = '';
let eventIdString = '';
const { container } = render(
<TestApp
fallback={({ error, componentStack }) => {
if (error && componentStack) {
fallback={({ error, componentStack, eventId }) => {
if (error && componentStack && eventId) {
errorString = error.toString();
compStack = componentStack;
eventIdString = eventId;
}
return <div>Fallback here</div>;
}}
Expand All @@ -167,6 +169,7 @@ describe('ErrorBoundary', () => {
in Bam (created by TestApp)
in ErrorBoundary (created by TestApp)
in TestApp`);
expect(eventIdString).toBe(EVENT_ID);
});
});

Expand All @@ -186,7 +189,7 @@ describe('ErrorBoundary', () => {
fireEvent.click(btn);

expect(mockOnError).toHaveBeenCalledTimes(1);
expect(mockOnError).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
expect(mockOnError).toHaveBeenCalledWith(expect.any(Error), expect.any(String), expect.any(String));

expect(mockCaptureException).toHaveBeenCalledTimes(1);
expect(mockCaptureException).toHaveBeenCalledWith(expect.any(Error), {
Expand Down Expand Up @@ -249,7 +252,7 @@ describe('ErrorBoundary', () => {
fireEvent.click(reset);

expect(mockOnReset).toHaveBeenCalledTimes(1);
expect(mockOnReset).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
expect(mockOnReset).toHaveBeenCalledWith(expect.any(Error), expect.any(String), expect.any(String));
});
});
});