diff --git a/app/components/markdown/error_boundary.tsx b/app/components/markdown/error_boundary.tsx index 0dffd3d79..282e22a95 100644 --- a/app/components/markdown/error_boundary.tsx +++ b/app/components/markdown/error_boundary.tsx @@ -27,7 +27,7 @@ const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => ({ }, })); -class ErrorBoundary extends React.PureComponent { +class ErrorBoundary extends React.PureComponent { constructor(props: Props) { super(props); this.state = {hasError: false}; @@ -38,7 +38,6 @@ class ErrorBoundary extends React.PureComponent { } render() { - // eslint-disable-next-line react/prop-types const {children, error, theme} = this.props; const {hasError} = this.state; const style = getStyleSheet(theme); diff --git a/app/components/markdown/markdown.test.tsx b/app/components/markdown/markdown.test.tsx new file mode 100644 index 000000000..804a36916 --- /dev/null +++ b/app/components/markdown/markdown.test.tsx @@ -0,0 +1,140 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {screen} from '@testing-library/react-native'; +import * as CommonMark from 'commonmark'; +const {Node} = CommonMark; +import React from 'react'; + +import {Preferences} from '@constants'; +import {renderWithIntl} from '@test/intl-test-helper'; + +import Markdown from './markdown'; +import * as Transforms from './transform'; + +const Parser = jest.requireActual('commonmark').Parser; + +describe('Markdown', () => { + const baseProps: React.ComponentProps = { + baseTextStyle: {}, + enableInlineLatex: true, + enableLatex: true, + location: 'somewhere?', + maxNodes: 2000, + theme: Preferences.THEMES.denim, + }; + + describe('error handling', () => { + test('should render Markdown normally', () => { + renderWithIntl( + , + ); + + expect(screen.getByText('This is a test')).toBeVisible(); + }); + + test('should catch errors when parsing Markdown', () => { + jest.spyOn(CommonMark, 'Parser').mockImplementation(() => { + const parser = new Parser(); + parser.incorporateLine = () => { + throw new Error('test error'); + }; + return parser; + }); + + renderWithIntl( + , + ); + + expect(screen.queryByText('This is a text')).not.toBeVisible(); + expect(screen.getByText('An error occurred while parsing this text')).toBeVisible(); + }); + + test('should catch errors when parsing Markdown', () => { + jest.spyOn(CommonMark, 'Parser').mockImplementation(() => { + const parser = new Parser(); + parser.incorporateLine = () => { + throw new Error('test error'); + }; + return parser; + }); + + renderWithIntl( + , + ); + + expect(screen.queryByText('This is a text')).not.toBeVisible(); + expect(screen.getByText('An error occurred while parsing this text')).toBeVisible(); + }); + + test('should catch errors when transforming Markdown', () => { + jest.spyOn(Transforms, 'highlightMentions').mockImplementation(() => { + throw new Error('test error'); + }); + + renderWithIntl( + , + ); + + expect(screen.queryByText('This is a text')).not.toBeVisible(); + expect(screen.getByText('An error occurred while parsing this text')).toBeVisible(); + }); + + test('should catch errors when rendering Markdown', () => { + jest.spyOn(CommonMark, 'Parser').mockImplementation(() => { + const parser = new Parser(); + parser.parse = () => { + // This replicates what was returned by the parser to cause MM-61148 + const document = new Node('document'); + + const paragraph = new Node('paragraph'); + (paragraph as any)._string_content = 'some text'; + + document.appendChild(new Node('table')); + + return document; + }; + return parser; + }); + + renderWithIntl( + , + ); + + expect(screen.queryByText('This is a text')).not.toBeVisible(); + expect(screen.getByText('An error occurred while rendering this text')).toBeVisible(); + }); + + test('should catch errors when with Latex', () => { + const value = + '```latex\n' + + '\\\\\\\\asdfasdfasdf\n' + + '```\n'; + + renderWithIntl( + , + ); + + expect(screen.queryByText('This is a text')).not.toBeVisible(); + expect(screen.getByText('An error occurred while rendering this text')).toBeVisible(); + }); + }); +}); diff --git a/app/components/markdown/markdown.tsx b/app/components/markdown/markdown.tsx index c507797ea..934e7cdfb 100644 --- a/app/components/markdown/markdown.tsx +++ b/app/components/markdown/markdown.tsx @@ -10,8 +10,10 @@ import {Dimensions, type GestureResponderEvent, Platform, type StyleProp, StyleS import CompassIcon from '@components/compass_icon'; import Emoji from '@components/emoji'; import FormattedText from '@components/formatted_text'; +import {logError} from '@utils/log'; import {computeTextStyle} from '@utils/markdown'; import {blendColors, changeOpacity, concatStyles, makeStyleSheetFromTheme} from '@utils/theme'; +import {typography} from '@utils/typography'; import {getScheme} from '@utils/url'; import AtMention from './at_mention'; @@ -98,6 +100,10 @@ const getStyleSheet = makeStyleSheetFromTheme((theme) => { color: editedColor, opacity: editedOpacity, }, + errorMessage: { + color: theme.errorTextColor, + ...typography('Body', 100), + }, maxNodesWarning: { color: theme.errorTextColor, }, @@ -606,34 +612,74 @@ const Markdown = ({ const parser = useRef(new Parser({urlFilter, minimumHashtagLength})).current; const renderer = useMemo(createRenderer, [theme, textStyles]); - let ast = parser.parse(value.toString()); - ast = combineTextNodes(ast); - ast = addListItemIndices(ast); - ast = pullOutImages(ast); - ast = parseTaskLists(ast); - if (mentionKeys) { - ast = highlightMentions(ast, mentionKeys); - } - if (highlightKeys) { - ast = highlightWithoutNotification(ast, highlightKeys); - } - if (searchPatterns) { - ast = highlightSearchPatterns(ast, searchPatterns); - } - if (isEdited) { - const editIndicatorNode = new Node('edited_indicator'); - if (ast.lastChild && ['heading', 'paragraph'].includes(ast.lastChild.type)) { - ast.appendChild(editIndicatorNode); - } else { - const node = new Node('paragraph'); - node.appendChild(editIndicatorNode); + const errorLogged = useRef(false); - ast.appendChild(node); + let ast; + try { + ast = parser.parse(value.toString()); + + ast = combineTextNodes(ast); + ast = addListItemIndices(ast); + ast = pullOutImages(ast); + ast = parseTaskLists(ast); + if (mentionKeys) { + ast = highlightMentions(ast, mentionKeys); } + if (highlightKeys) { + ast = highlightWithoutNotification(ast, highlightKeys); + } + if (searchPatterns) { + ast = highlightSearchPatterns(ast, searchPatterns); + } + + if (isEdited) { + const editIndicatorNode = new Node('edited_indicator'); + if (ast.lastChild && ['heading', 'paragraph'].includes(ast.lastChild.type)) { + ast.appendChild(editIndicatorNode); + } else { + const node = new Node('paragraph'); + node.appendChild(editIndicatorNode); + + ast.appendChild(node); + } + } + } catch (e) { + if (!errorLogged.current) { + logError('An error occurred while parsing Markdown', e); + + errorLogged.current = true; + } + + return ( + + ); } - return renderer.render(ast) as JSX.Element; + let output; + try { + output = renderer.render(ast); + } catch (e) { + if (!errorLogged.current) { + logError('An error occurred while rendering Markdown', e); + + errorLogged.current = true; + } + + return ( + + ); + } + + return output; }; export default Markdown; diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 7741aebb8..76f2e399e 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -454,6 +454,8 @@ "login.username": "Username", "markdown.latex.error": "Latex render error", "markdown.max_nodes.error": "This message is too long to by shown fully on a mobile device. Please view it on desktop or contact an admin to increase this limit.", + "markdown.parse_error": "An error occurred while parsing this text", + "markdown.render_error": "An error occurred while rendering this text", "mentions.empty.paragraph": "You'll see messages here when someone mentions you or uses terms you're monitoring.", "mentions.empty.title": "No Mentions yet", "mobile.about.appVersion": "App Version: {version} (Build {number})", diff --git a/package-lock.json b/package-lock.json index 7256e058e..f52d43990 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,7 +42,7 @@ "@stream-io/flat-list-mvcp": "0.10.3", "@voximplant/react-native-foreground-service": "3.0.2", "base-64": "1.0.0", - "commonmark": "npm:@mattermost/commonmark@0.30.1-3", + "commonmark": "npm:@mattermost/commonmark@0.30.1-4", "commonmark-react-renderer": "github:mattermost/commonmark-react-renderer#81b5d27509652bae50b4b510ede777dd3bd923cf", "deep-equal": "2.2.3", "deepmerge": "4.3.1", @@ -12057,9 +12057,9 @@ }, "node_modules/commonmark": { "name": "@mattermost/commonmark", - "version": "0.30.1-3", - "resolved": "https://registry.npmjs.org/@mattermost/commonmark/-/commonmark-0.30.1-3.tgz", - "integrity": "sha512-Kjsl/sZmb6R6PtpPVIifPfqBMrKs7z6Tukb1TJl/S0LfC5uNici3yol4waGxhGsJuvCF2kZqStwVQP7ieUbAgw==", + "version": "0.30.1-4", + "resolved": "https://registry.npmjs.org/@mattermost/commonmark/-/commonmark-0.30.1-4.tgz", + "integrity": "sha512-7O1xQfP3ILHf/+wjrxwF0rqowuvqQ7EY+SJOWsDaV+VXdj+lmft9yxkL0QPNYALinKFdPEj1gFtgR+pi4w8lzQ==", "dependencies": { "entities": "~3.0.1", "mdurl": "~1.0.1", diff --git a/package.json b/package.json index c8fd07de0..3ca3e2a2a 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "@stream-io/flat-list-mvcp": "0.10.3", "@voximplant/react-native-foreground-service": "3.0.2", "base-64": "1.0.0", - "commonmark": "npm:@mattermost/commonmark@0.30.1-3", + "commonmark": "npm:@mattermost/commonmark@0.30.1-4", "commonmark-react-renderer": "github:mattermost/commonmark-react-renderer#81b5d27509652bae50b4b510ede777dd3bd923cf", "deep-equal": "2.2.3", "deepmerge": "4.3.1",