Skip to content

Commit

Permalink
remove unhandledrejection listener and provide setError in a context (#…
Browse files Browse the repository at this point in the history
…21)

* remove unhandledrejection listener and provide setError in a context

Listening to all unhandledrejection events is not ideal for setting the
error screen, as these events can also come from non-app code, which
leads to the error screen displaying for non-issues from third-party
code. This updates the ErrorBoundary to remove the listener
completely, and provide setError as the preferred method for handling
async errors where they occur.

* add comment describing hook

* this did not need to be renamed

* notify sentry within the hook

* tidy

* export the error context and hook

* set the event id from captureException
  • Loading branch information
jivey authored Oct 18, 2023
1 parent 42ab197 commit 37f0f76
Show file tree
Hide file tree
Showing 15 changed files with 198 additions and 230 deletions.
6 changes: 3 additions & 3 deletions src/components/Error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const Error = ({ heading, children, ...props }: ErrorPropTypes) => {
const [lastEventId, setLastEventId] = React.useState<string | undefined>(Sentry.lastEventId());

React.useEffect(() => {
if (context?.eventId || lastEventId) {
if (context.error?.eventId || lastEventId) {
return;
}

Expand All @@ -34,14 +34,14 @@ export const Error = ({ heading, children, ...props }: ErrorPropTypes) => {
}, 100);

return () => clearInterval(intervalId);
}, [lastEventId, context?.eventId]);
}, [lastEventId, context.error?.eventId]);

return <ModalBody {...props} data-testid='error'>
<ModalBodyHeading>{heading ?? `Uh-oh, there's been a glitch`}</ModalBodyHeading>
{children ?? <>
We're not quite sure what went wrong. Restart your browser. If this doesn't solve
the problem, visit our <a href="https://openstax.secure.force.com/help" target="_blank">Support Center</a>.
</>}
<EventId data-testid='event-id'>{context?.eventId || lastEventId}</EventId>
<EventId data-testid='event-id'>{context.error?.eventId || lastEventId}</EventId>
</ModalBody>
};
119 changes: 1 addition & 118 deletions src/components/ErrorBoundary.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import renderer, { ReactTestRenderer, act } from 'react-test-renderer';
import { ErrorBoundary, getTypeFromError } from './ErrorBoundary';
import { ErrorBoundary } from './ErrorBoundary';
import sentryTestkit from 'sentry-testkit';
import * as Sentry from '@sentry/react';
import { findByTestId } from '../test/utils';
Expand Down Expand Up @@ -96,123 +96,6 @@ describe('ErrorBoundary', () => {
spy.mockRestore();
});

describe('getTypeFromError', () => {
it('returns name if the constuctor does not contain TYPE ', () => {
class MyError extends Error {
constructor() {
super();
Object.setPrototypeOf(this, MyError.prototype);
}
}

expect(getTypeFromError(new MyError())).toEqual('MyError');
});
})

describe('unhandled rejections', () => {
it('are captured', async () => {
const callbacks: Record<string, ({ reason }: { reason: string }) => void> = {};
const mockWindow = {
addEventListener: jest.fn().mockImplementation(
(event, callback) => callbacks[event] = callback
),
removeEventListener: jest.fn().mockImplementation(
(event: string) => delete callbacks[event],
)
};

let render: ReactTestRenderer | undefined;
act(() => {
render = renderer.create(
<ErrorBoundary renderFallback windowImpl={mockWindow}>
Content
</ErrorBoundary>
);
});

expect(mockWindow.addEventListener).toHaveBeenCalled();

act(() => {
callbacks.unhandledrejection({ reason: 'Test Rejection' });
});

expect(render?.toJSON()).toMatchSnapshot();

act(() => {
render?.unmount();
});

expect(mockWindow.removeEventListener).toHaveBeenCalled();
});

it('can be disabled', async () => {
const callbacks: Record<string, ({ reason }: { reason: string }) => void> = {};
const mockWindow = {
addEventListener: jest.fn().mockImplementation(
(event, callback) => callbacks[event] = callback
),
removeEventListener: jest.fn().mockImplementation(
(event: string) => delete callbacks[event],
)
};

let render: ReactTestRenderer | undefined;
act(() => {
render = renderer.create(
<ErrorBoundary renderFallback windowImpl={mockWindow} catchUnhandledRejections={false}>
Content
</ErrorBoundary>
);
});

expect(mockWindow.addEventListener).not.toHaveBeenCalled();
expect(callbacks).toStrictEqual({});
expect(render?.toJSON()).toMatchSnapshot();

act(() => {
render?.unmount();
});

expect(mockWindow.removeEventListener).not.toHaveBeenCalled();
});

it('does not crash on undefined reasons', async () => {
const callbacks: Record<string, (_: unknown) => void> = {};

const mockWindow = {
addEventListener: jest.fn().mockImplementation(
(event, callback) => callbacks[event] = callback
),
removeEventListener: jest.fn().mockImplementation(
(event: string) => delete callbacks[event],
)
};

let render: ReactTestRenderer | undefined;
act(() => {
render = renderer.create(
<ErrorBoundary renderFallback windowImpl={mockWindow}>
Content
</ErrorBoundary>
);
});

expect(mockWindow.addEventListener).toHaveBeenCalled();

act(() => {
callbacks.unhandledrejection({});
});

expect(render?.toJSON()).toMatchSnapshot();

act(() => {
render?.unmount();
});

expect(mockWindow.removeEventListener).toHaveBeenCalled();
});
});

it('inits Sentry', () => {
jest.resetAllMocks();
const initSpy = jest.spyOn(Sentry, 'init');
Expand Down
41 changes: 29 additions & 12 deletions src/components/ErrorBoundary.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from "react";
import { ErrorBoundary } from "./ErrorBoundary";
import { ErrorMessage } from "./ErrorMessage";
import { UnauthorizedError, SessionExpiredError } from "@openstax/ts-utils/errors";
import { useSetAppError } from "../../src/hooks";

const ErrorComponent = ({ doThrow, setShowError, error: error, errorMessage }: {
doThrow: boolean;
Expand All @@ -22,6 +23,15 @@ const ErrorComponent = ({ doThrow, setShowError, error: error, errorMessage }: {
return null;
};

const AsyncTriggerErrorDisplayButton = () => {
const setAppError = useSetAppError();
return <button onClick={() => {
Promise.reject(Error('Test Error')).catch(setAppError);
}}>
Async Trigger Error Display
</button>;
};

export const InlineMessages = () => {
const [showError, setShowError] = React.useState(false);

Expand All @@ -32,9 +42,7 @@ export const InlineMessages = () => {
setShowError={setShowError}
/>
<button onClick={() => { setShowError(true) }}>Throw Error</button>
<button onClick={() => {
Promise.reject( Error('Test Error') )
}}>Reject Promise</button>
<AsyncTriggerErrorDisplayButton />
</ErrorBoundary>
};

Expand All @@ -47,9 +55,7 @@ export const Fallback_GenericError_Default = () => {
setShowError={setShowError}
/>
<button onClick={() => { setShowError(true) }}>Throw Error</button>
<button onClick={() => {
Promise.reject( Error('Test Error') );
}}>Reject Promise</button>
<AsyncTriggerErrorDisplayButton />
</ErrorBoundary>
};

Expand All @@ -70,12 +76,25 @@ export const Fallback_GenericError_Custom = () => {
setShowError={setShowError}
/>
<button onClick={() => { setShowError(true) }}>Throw Error</button>
<button onClick={() => {
Promise.reject( Error('Test Error') )
}}>Reject Promise</button>
<AsyncTriggerErrorDisplayButton />
</ErrorBoundary>
};

const AsyncSessionExpiredButton = () => {
const setAppError = useSetAppError();

return <button onClick={() => {
(async () => {
try {
throw new SessionExpiredError();
} catch (e: any) {
setAppError(e);
}
})();
}}>
Async Throw Session Expired
</button>;
};
export const Fallback_SpecialError = () => {
const [showError1, setShowError1] = React.useState(false);
const [showError2, setShowError2] = React.useState(false);
Expand All @@ -95,9 +114,7 @@ export const Fallback_SpecialError = () => {
/>
<button onClick={() => { setShowError1(true) }}>Throw SessionExpiredError</button>
<br />
<button onClick={async () => {
throw new SessionExpiredError();
}}>Throw Async SessionExpiredError</button>
<AsyncSessionExpiredButton />
<br />
<button onClick={() => { setShowError2(true) }}>Throw UnauthorizedError</button>
</ErrorBoundary>
Expand Down
39 changes: 6 additions & 33 deletions src/components/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Error as ErrorComponent, ErrorPropTypes } from './Error';
import type { ErrorBoundaryProps } from '@sentry/react/types/errorboundary';
import { ErrorContext } from '../contexts';
import { SentryError } from '../types';
import { getTypeFromError } from '../utils';

const Error = ({ children, ...props }: React.PropsWithChildren<ErrorPropTypes>) =>
<ErrorComponent data-testid='error-fallback' {...props}>{children}</ErrorComponent>;
Expand All @@ -19,30 +20,20 @@ const defaultErrorFallbacks = {
</Error>
};

export const getTypeFromError = (error: Error | PromiseRejectionEvent['reason']) => {
if (!error) { return undefined; }
const { TYPE, name } = error.constructor;
return TYPE && typeof TYPE === 'string' ? TYPE : name;
};

export const ErrorBoundary = ({
children,
renderFallback,
fallback = defaultErrorFallbacks['generic'],
catchUnhandledRejections = true,
windowImpl = window,
sentryDsn,
sentryInit,
...props
}: ErrorBoundaryProps & {
renderFallback?: boolean;
catchUnhandledRejections?: boolean;
windowImpl?: Window | Pick<Window, 'addEventListener' | 'removeEventListener'>;
sentryDsn?: string;
sentryInit?: Sentry.BrowserOptions;
errorFallbacks?: { [_: string]: JSX.Element }
}) => {
const [error, setError] = React.useState<SentryError & { type?: string } | null>(null);
const [error, setError] = React.useState<SentryError | null>(null);
const errorFallbacks = { ...defaultErrorFallbacks, ...props.errorFallbacks };
const typedFallback = error?.type ? errorFallbacks[error.type] : undefined;

Expand All @@ -63,28 +54,10 @@ export const ErrorBoundary = ({
});
}, []);

React.useEffect(() => {
if (!catchUnhandledRejections) {
return;
}

const handleRejection = (e: PromiseRejectionEvent) => {
setError({
error: {
name: e.type,
message: e.reason?.toString(),
},
type: getTypeFromError(e.reason)
});
};

windowImpl.addEventListener('unhandledrejection', handleRejection);
return () => windowImpl.removeEventListener('unhandledrejection', handleRejection);
}, []);
// There are two references to the render element here because the Sentry fallback (and onError)
// are not used for unhandledrejection events. To support those events, we add a listener and
// reuse the same error state and rendering logic.
return <ErrorContext.Provider value={error}>
// There are two references to the render element here because the Sentry fallback (and
// onError) are not used for unhandledrejection events. To support those events, we provide
// setError in a context to reuse the same error state and render logic.
return <ErrorContext.Provider value={{ error, setError }}>
<Sentry.ErrorBoundary
fallback={renderElement}
onError={(error, componentStack, eventId) => {
Expand Down
6 changes: 3 additions & 3 deletions src/components/ErrorMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ export const ErrorMessage = ({ message, showEventId = true }: {
message?: string;
showEventId?: boolean;
}) => {
const context = React.useContext(ErrorContext);
const { error } = React.useContext(ErrorContext);

return <>
{message || context?.error.message}
{showEventId ? <div>{context?.eventId}</div> : null}
{message || error?.error?.message }
{showEventId ? <div>{error?.eventId}</div> : null}
</>
};
6 changes: 4 additions & 2 deletions src/components/ErrorModal.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ export const Default = () => {
Promise.reject('Test error for modal');
}, []);

const [show, setShow] = React.useState(true);

return <ErrorBoundary sentryDsn="https://[email protected]/0">
<ErrorModal
onModalClose={() => undefined}
show={true}
onModalClose={() => setShow(false)}
show={show}
/>
</ErrorBoundary>;
}
Loading

0 comments on commit 37f0f76

Please sign in to comment.