From 5da717190381f57baa671638d340c6585da7df86 Mon Sep 17 00:00:00 2001 From: Max Falk Date: Sun, 6 Jun 2021 21:16:01 +0200 Subject: [PATCH 1/4] fix(cli): fix TypeError when formatting error messages error.errors can be undefined which will lead to a TypeError, swallowing the actual error message in the process Signed-off-by: Max Falk --- packages/cli/src/lib/builder/packager.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lib/builder/packager.ts b/packages/cli/src/lib/builder/packager.ts index ed76e23df3..2c85ec3df3 100644 --- a/packages/cli/src/lib/builder/packager.ts +++ b/packages/cli/src/lib/builder/packager.ts @@ -28,15 +28,17 @@ function formatErrorMessage(error: any) { if (error.code === 'PLUGIN_ERROR') { if (error.plugin === 'esbuild') { msg += `${error.message}\n\n`; - for (const { text, location } of error.errors) { - const { line, column } = location; - const path = relativePath(paths.targetDir, error.id); - const loc = chalk.cyan(`${path}:${line}:${column}`); + if (error.errors) { + for (const { text, location } of error.errors) { + const { line, column } = location; + const path = relativePath(paths.targetDir, error.id); + const loc = chalk.cyan(`${path}:${line}:${column}`); - if (text === 'Unexpected "<"' && error.id.endsWith('.js')) { - msg += `${loc}: ${text}, JavaScript files with JSX should use a .jsx extension`; - } else { - msg += `${loc}: ${text}`; + if (text === 'Unexpected "<"' && error.id.endsWith('.js')) { + msg += `${loc}: ${text}, JavaScript files with JSX should use a .jsx extension`; + } else { + msg += `${loc}: ${text}`; + } } } } else { From f3a3134f7e865a505afc1c27b3d479e21d809c06 Mon Sep 17 00:00:00 2001 From: Max Falk Date: Sun, 6 Jun 2021 22:26:28 +0200 Subject: [PATCH 2/4] fix(cli): fix TypeError when formatting error messages error.errors can be undefined which will lead to a TypeError, swallowing the actual error message in the process Signed-off-by: Max Falk --- packages/cli/src/lib/builder/packager.test.ts | 28 +++++++++++++++++++ packages/cli/src/lib/builder/packager.ts | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/lib/builder/packager.test.ts diff --git a/packages/cli/src/lib/builder/packager.test.ts b/packages/cli/src/lib/builder/packager.test.ts new file mode 100644 index 0000000000..69f4b4e524 --- /dev/null +++ b/packages/cli/src/lib/builder/packager.test.ts @@ -0,0 +1,28 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { formatErrorMessage } from './packager'; + +describe('formatErrorMessage with esbuild plugin error', () => { + it('given error with missing errors array then error message should be shown', () => { + const msg = formatErrorMessage({ + code: 'PLUGIN_ERROR', + plugin: 'esbuild', + message: 'test', + }); + expect(msg).toBe('test\n\n'); + }); +}); diff --git a/packages/cli/src/lib/builder/packager.ts b/packages/cli/src/lib/builder/packager.ts index 2c85ec3df3..d6519e79a9 100644 --- a/packages/cli/src/lib/builder/packager.ts +++ b/packages/cli/src/lib/builder/packager.ts @@ -22,7 +22,7 @@ import { paths } from '../paths'; import { makeConfigs } from './config'; import { BuildOptions } from './types'; -function formatErrorMessage(error: any) { +export function formatErrorMessage(error: any) { let msg = ''; if (error.code === 'PLUGIN_ERROR') { From 21e8ebef5d4b9bb6df4211cc489b442f7d67a224 Mon Sep 17 00:00:00 2001 From: Max Falk Date: Sun, 6 Jun 2021 22:41:17 +0200 Subject: [PATCH 3/4] add changeset Signed-off-by: Max Falk --- .changeset/sour-brooms-dream.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/sour-brooms-dream.md diff --git a/.changeset/sour-brooms-dream.md b/.changeset/sour-brooms-dream.md new file mode 100644 index 0000000000..4cb5a0e584 --- /dev/null +++ b/.changeset/sour-brooms-dream.md @@ -0,0 +1,9 @@ +--- +'@backstage/cli': patch +--- + +Fix error message formatting in the packaging process. + +error.errors can be undefined which will lead to a TypeError, swallowing the actual error message in the process. + +For instance, if you break your tsconfig.json with invalid syntax, backstage-cli will not be able to build anything and it will be very hard to find out why because the underlying error message is hidden behind a TypeError. From 30627bd3e7db62b7469bdf9cbb3981dd17f2c4a5 Mon Sep 17 00:00:00 2001 From: Max Falk Date: Mon, 7 Jun 2021 23:51:34 +0200 Subject: [PATCH 4/4] check array length and only add newlines if length > 0 Signed-off-by: Max Falk --- packages/cli/src/lib/builder/packager.test.ts | 12 +++++++++++- packages/cli/src/lib/builder/packager.ts | 5 +++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/lib/builder/packager.test.ts b/packages/cli/src/lib/builder/packager.test.ts index 69f4b4e524..f5e101cfc6 100644 --- a/packages/cli/src/lib/builder/packager.test.ts +++ b/packages/cli/src/lib/builder/packager.test.ts @@ -23,6 +23,16 @@ describe('formatErrorMessage with esbuild plugin error', () => { plugin: 'esbuild', message: 'test', }); - expect(msg).toBe('test\n\n'); + expect(msg).toBe('test'); + }); + it('given error with errors array then error message should have new lines', () => { + const msg = formatErrorMessage({ + code: 'PLUGIN_ERROR', + plugin: 'esbuild', + message: 'test', + id: 'index.js', + errors: [{ text: 'Dummy', location: { line: 1, column: 1 } }], + }); + expect(msg).toContain('test\n\n'); }); }); diff --git a/packages/cli/src/lib/builder/packager.ts b/packages/cli/src/lib/builder/packager.ts index d6519e79a9..6deded5f69 100644 --- a/packages/cli/src/lib/builder/packager.ts +++ b/packages/cli/src/lib/builder/packager.ts @@ -27,8 +27,9 @@ export function formatErrorMessage(error: any) { if (error.code === 'PLUGIN_ERROR') { if (error.plugin === 'esbuild') { - msg += `${error.message}\n\n`; - if (error.errors) { + msg += `${error.message}`; + if (error.errors?.length) { + msg += `\n\n`; for (const { text, location } of error.errors) { const { line, column } = location; const path = relativePath(paths.targetDir, error.id);