From c7eb908e386b2ffc3bbc7726f9fb54820fd8c48e Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 20 Jan 2025 10:02:11 -0500 Subject: [PATCH] MM-61734 Add withErrorBoundary HOC and use for PostComponent (#29801) * MM-61734 Add withErrorBoundary HOC and use for PostComponent I originally planned to add error boundaries to some individual parts of the post, but it seems sufficient to add them to just the outer PostComponent. * Update button label --- .../src/components/post/post_component.tsx | 7 +- .../components/post/post_error_boundary.tsx | 43 ++++++ .../__snapshots__/post_list_row.test.tsx.snap | 4 +- .../with_error_boundary/index.test.tsx | 138 ++++++++++++++++++ .../components/with_error_boundary/index.tsx | 54 +++++++ webapp/channels/src/i18n/en.json | 3 + 6 files changed, 244 insertions(+), 5 deletions(-) create mode 100644 webapp/channels/src/components/post/post_error_boundary.tsx create mode 100644 webapp/channels/src/components/with_error_boundary/index.test.tsx create mode 100644 webapp/channels/src/components/with_error_boundary/index.tsx diff --git a/webapp/channels/src/components/post/post_component.tsx b/webapp/channels/src/components/post/post_component.tsx index 54e740cca9..cc4b303960 100644 --- a/webapp/channels/src/components/post/post_component.tsx +++ b/webapp/channels/src/components/post/post_component.tsx @@ -49,6 +49,7 @@ import {getDateForUnixTicks, makeIsEligibleForClick} from 'utils/utils'; import type {PostActionComponent, PostPluginComponent} from 'types/store/plugins'; +import {withPostErrorBoundary} from './post_error_boundary'; import PostOptions from './post_options'; import PostUserProfile from './user_profile'; @@ -119,7 +120,7 @@ export type Props = { pluginActions: PostActionComponent[]; }; -const PostComponent = (props: Props): JSX.Element => { +function PostComponent(props: Props) { const {post, shouldHighlight, togglePostMenu} = props; const isSearchResultItem = (props.matches && props.matches.length > 0) || props.isMentionSearch || (props.term && props.term.length > 0); @@ -670,6 +671,6 @@ const PostComponent = (props: Props): JSX.Element => { ); -}; +} -export default PostComponent; +export default withPostErrorBoundary(PostComponent); diff --git a/webapp/channels/src/components/post/post_error_boundary.tsx b/webapp/channels/src/components/post/post_error_boundary.tsx new file mode 100644 index 0000000000..0728987c6e --- /dev/null +++ b/webapp/channels/src/components/post/post_error_boundary.tsx @@ -0,0 +1,43 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; +import {FormattedMessage, useIntl} from 'react-intl'; + +import type {FallbackProps} from 'components/with_error_boundary'; +import withErrorBoundary from 'components/with_error_boundary'; + +export function withPostErrorBoundary

(component: React.ComponentType

) { + return withErrorBoundary

(component, { + renderFallback: ({clearError}) => { + return ( +

+ +
+ +
+ ); + }, + }); +} + +function RetryButton({clearError}: FallbackProps) { + const intl = useIntl(); + + return ( + + ); +} diff --git a/webapp/channels/src/components/post_view/post_list_row/__snapshots__/post_list_row.test.tsx.snap b/webapp/channels/src/components/post_view/post_list_row/__snapshots__/post_list_row.test.tsx.snap index b402d40698..4f6153e8b8 100644 --- a/webapp/channels/src/components/post_view/post_list_row/__snapshots__/post_list_row.test.tsx.snap +++ b/webapp/channels/src/components/post_view/post_list_row/__snapshots__/post_list_row.test.tsx.snap @@ -43,7 +43,7 @@ exports[`components/post_view/post_list_row should have class hideAnimation for exports[`components/post_view/post_list_row should render channel intro message 1`] = ``; exports[`components/post_view/post_list_row should render combined post 1`] = ` - +

{'A rendering error occurred'}

+ + + ); +} + +describe('withErrorBoundary', () => { + const origError = console.error; + beforeAll(() => { + console.error = jest.fn(); + }); + afterAll(() => { + console.error = origError; + }); + + test('should render the component normally', () => { + function TestComponent() { + return {'TestComponent'}; + } + const WrappedTestComponent = withErrorBoundary(TestComponent, { + renderFallback: renderFallbackWithRetry, + }); + + render( + , + ); + + expect(screen.getByText('TestComponent')).toBeVisible(); + }); + + test('should render fallback when an error occurs during rendering', () => { + function TestComponent(): JSX.Element { + const obj = {} as any; + + return {'TestComponent' + obj.someField.thatDoesnt.exist.toString()}; + } + const WrappedTestComponent = withErrorBoundary(TestComponent, { + renderFallback: renderFallbackWithRetry, + }); + + render( + , + ); + + expect(screen.getByText('A rendering error occurred')).toBeVisible(); + }); + + test('should render fallback when an error occurs in a hook', () => { + function useAnErrorForSomeReason(): string { + throw new Error('hook error'); + } + function TestComponent(): JSX.Element { + const extraText = useAnErrorForSomeReason(); + + return {'TestComponent' + extraText}; + } + const WrappedTestComponent = withErrorBoundary(TestComponent, { + renderFallback: renderFallbackWithRetry, + }); + + render( + , + ); + + expect(screen.getByText('A rendering error occurred')).toBeVisible(); + }); + + test('should render fallback when an error occurs in a selector', () => { + function TestComponent(): JSX.Element { + const extraText = useSelector(getChannelsInCategoryOrder); + + return {'TestComponent' + extraText}; + } + const WrappedTestComponent = withErrorBoundary(TestComponent, { + renderFallback: renderFallbackWithRetry, + }); + + render( + , + ); + + expect(screen.getByText('A rendering error occurred')).toBeVisible(); + }); + + test('the user should be able to retry rendering the component', () => { + let throwError = true; + + function TestComponent(): JSX.Element { + let obj: any; + if (throwError) { + obj = {}; + } else { + obj = { + someField: { + thatDoesnt: { + exist: [1, 2, 3], + }, + }, + }; + } + + return {'TestComponent ' + obj.someField.thatDoesnt.exist.toString()}; + } + const WrappedTestComponent = withErrorBoundary(TestComponent, { + renderFallback: renderFallbackWithRetry, + }); + + render( + , + ); + + expect(screen.queryByText('A rendering error occurred')).toBeVisible(); + expect(screen.queryByText('TestComponent 1,2,3')).toBeNull(); + + throwError = false; + + screen.getByText('Try again?').click(); + + expect(screen.queryByText('A rendering error occurred')).toBeNull(); + expect(screen.queryByText('TestComponent 1,2,3')).toBeVisible(); + }); +}); diff --git a/webapp/channels/src/components/with_error_boundary/index.tsx b/webapp/channels/src/components/with_error_boundary/index.tsx new file mode 100644 index 0000000000..d0aad13951 --- /dev/null +++ b/webapp/channels/src/components/with_error_boundary/index.tsx @@ -0,0 +1,54 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +type ErrorBoundaryState = { + hasError: boolean; +} + +export type FallbackProps = { + clearError: (e: React.MouseEvent) => void; +}; + +type ErrorBoundaryOptions = { + renderFallback: (props: FallbackProps) => React.ReactNode; +}; + +export default function withErrorBoundary

(component: React.ComponentType

, options: ErrorBoundaryOptions) { + const Component = component; + const displayName = component.displayName ?? component.name ?? 'Component'; + + const WrappedComponent = class WrappedComponent extends React.PureComponent { + static displayName = `ErrorBoundary(${displayName})`; + + state = { + hasError: false, + }; + + static getDerivedStateFromError() { + return { + hasError: true, + }; + } + + clearError = (e: React.MouseEvent) => { + e.preventDefault(); + e.stopPropagation(); + + this.setState({hasError: false}); + }; + + render() { + if (this.state.hasError) { + return options.renderFallback({ + clearError: this.clearError, + }); + } + + return ; + } + }; + + return WrappedComponent; +} diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index f72bb7968a..694604d177 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -4731,6 +4731,9 @@ "post.ariaLabel.replyMessage": "At {time} {date}, {authorName} replied, {message}", "post.reminder.acknowledgement": "You will be reminded at {reminderTime}, {reminderDate} about this message from {username}: {permaLink}", "post.reminder.systemBot": "Hi there, here's your reminder about this message from {username}: {permaLink}", + "post.renderError.message": "An error occurred while rendering this post.", + "post.renderError.retry": "Retry", + "post.renderError.retryLabel": "Retry rendering this post", "postlist.toast.history": "Viewing message history", "postlist.toast.newMessages": "{count, number} new {count, plural, one {message} other {messages}}", "postlist.toast.newMessagesSince": "{count, number} new {count, plural, one {message} other {messages}} {isToday, select, true {} other {since}} {date}",