From fd01cdec71067de25ea572e0462ff710e8f8ef22 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gr=C3=A9goire=20Aubert?= Date: Tue, 18 Feb 2020 09:17:09 +0100 Subject: [PATCH] SONAR-13103 Parsing errors break the whole documentation app --- .../documentation/__tests__/pages-test.ts | 23 ++++++++++++++++++- .../src/main/js/apps/documentation/pages.ts | 15 ++++++++++-- .../__snapshots__/markdown-test.ts.snap | 12 ++++++++++ .../js/helpers/__tests__/markdown-test.ts | 20 ++++++++++++++++ .../sonar-web/src/main/js/helpers/markdown.js | 19 ++++++++++++++- 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/documentation/__tests__/pages-test.ts b/server/sonar-web/src/main/js/apps/documentation/__tests__/pages-test.ts index 2a7a6a07de9..822033a6e4a 100644 --- a/server/sonar-web/src/main/js/apps/documentation/__tests__/pages-test.ts +++ b/server/sonar-web/src/main/js/apps/documentation/__tests__/pages-test.ts @@ -17,7 +17,8 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import { ParsedContent } from '../../../helpers/markdown'; +/* eslint-disable no-console */ +import { filterContent, ParsedContent } from '../../../helpers/markdown'; import { mockDocumentationMarkdown } from '../../../helpers/testMocks'; jest.mock('remark', () => ({ @@ -32,6 +33,11 @@ jest.mock('unist-util-visit', () => ({ } })); +jest.mock('../../../helpers/markdown', () => { + const markdown = jest.requireActual('../../../helpers/markdown'); + return { ...markdown, filterContent: jest.fn().mockImplementation(markdown.filterContent) }; +}); + const lorem = { url: 'analysis/languages/lorem', title: 'toto doc', @@ -99,6 +105,21 @@ it('should correctly handle overrides (replace & add)', () => { expect(pages[2].title).toBe(newDoc.title); }); +it('should not break the whole doc when one page cannot be parsed', () => { + const originalConsoleError = console.error; + console.error = jest.fn(); + + (filterContent as jest.Mock).mockImplementationOnce(() => { + throw Error('Parse page error'); + }); + const pages = getPages(); + expect(pages.length).toBe(2); + expect(pages[0].content).toBe(''); + expect(console.error).toBeCalledTimes(1); + + console.error = originalConsoleError; +}); + function getPages(overrides: T.Dict = {}) { // This allows the use of out-of-scope data inside jest.mock // Usually, it is impossible as jest.mock'ed module is hoisted on the top of the file diff --git a/server/sonar-web/src/main/js/apps/documentation/pages.ts b/server/sonar-web/src/main/js/apps/documentation/pages.ts index 9ca8b4969d8..1d311ebd1bb 100644 --- a/server/sonar-web/src/main/js/apps/documentation/pages.ts +++ b/server/sonar-web/src/main/js/apps/documentation/pages.ts @@ -52,8 +52,19 @@ export default function getPages( }); return pages.map(({ parsed, file }) => { - const content = filterContent(parsed.content); - const text = getText(content); + let content = ''; + let text = ''; + try { + content = filterContent(parsed.content); + text = getText(content); + } catch (e) { + /* eslint-disable-next-line no-console */ + console.error( + `Documentation - an error occured while parsing page "${parsed.frontmatter.url || + file.path}":`, + e + ); + } return { relativeName: file.path, diff --git a/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/markdown-test.ts.snap b/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/markdown-test.ts.snap index 4d075b242b0..74dd4b87a03 100644 --- a/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/markdown-test.ts.snap +++ b/server/sonar-web/src/main/js/helpers/__tests__/__snapshots__/markdown-test.ts.snap @@ -55,3 +55,15 @@ Duis sagittis semper sapien nec tempor. Nullam vehicula nisi vitae nisi interdum " `; + +exports[`should not break when conditional tags are misused 1`] = ` +"Random SC text + Break + Bad SQ conditional formatting + Break + SC text + Break + Bad SQ conditional formatting + Break + Static stuff" +`; diff --git a/server/sonar-web/src/main/js/helpers/__tests__/markdown-test.ts b/server/sonar-web/src/main/js/helpers/__tests__/markdown-test.ts index 63d2fc3d8fd..dddfbf79beb 100644 --- a/server/sonar-web/src/main/js/helpers/__tests__/markdown-test.ts +++ b/server/sonar-web/src/main/js/helpers/__tests__/markdown-test.ts @@ -17,6 +17,7 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +/* eslint-disable no-console */ import { filterContent, getFrontMatter, separateFrontMatter } from '../markdown'; import { isSonarCloud } from '../system'; @@ -161,3 +162,22 @@ Duis sagittis semper sapien nec tempor. Nullam vehicula nisi vitae nisi interdum (isSonarCloud as jest.Mock).mockReturnValueOnce(true); expect(filterContent(content)).toMatchSnapshot(); }); + +it.only('should not break when conditional tags are misused', () => { + const originalConsoleError = console.error; + console.error = jest.fn(); + + const content = `Random SC text + Break + Bad SQ conditional formatting + Break + SCSQ text + Break + Bad SQ conditional formatting + Break + Static stuff`; + expect(filterContent(content)).toMatchSnapshot(); + expect(console.error).toBeCalledTimes(2); + + console.error = originalConsoleError; +}); diff --git a/server/sonar-web/src/main/js/helpers/markdown.js b/server/sonar-web/src/main/js/helpers/markdown.js index 2b717a1d7a1..276063f800d 100644 --- a/server/sonar-web/src/main/js/helpers/markdown.js +++ b/server/sonar-web/src/main/js/helpers/markdown.js @@ -96,7 +96,24 @@ function cutConditionalContent(content, tag) { let start = newContent.indexOf(beginning); let end = newContent.indexOf(ending); while (start !== -1 && end !== -1) { - newContent = newContent.substring(0, start) + newContent.substring(end + ending.length); + if (start < end) { + newContent = newContent.substring(0, start) + newContent.substring(end + ending.length); + } else { + // When conditional tags are incorrectly used we log an error, strip them out and pass to the next pair of tags + // eslint-disable-next-line no-console + console.error( + new Error( + `Documentation - incorrect usage of conditional formatting tags here: "${newContent.substring( + end, + start + beginning.length + )}"` + ) + ); + newContent = + newContent.substring(0, end) + + newContent.substring(end + ending.length, start) + + newContent.substring(start + beginning.length); + } start = newContent.indexOf(beginning); end = newContent.indexOf(ending); } -- 2.39.5