From 0acc3120ef46df957aaad93863eb234c1a6fac51 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Sat, 2 Nov 2019 05:53:13 -0700 Subject: [PATCH] cli: Handle errors when reading stdin (#6708) Co-authored-by: Simon Lydell --- CHANGELOG.unreleased.md | 28 +++++++++++++++++++++++++ package.json | 1 + src/cli/util.js | 36 +++++++++++++++----------------- tests_integration/runPrettier.js | 7 ++++--- yarn.lock | 5 +++++ 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index c772b439..bd578028 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -1292,6 +1292,31 @@ const example1 = (): { p: string => string } => (0: any); const example1 = (): ({ p: string => string }) => (0: any); ``` +#### CLI: Handle errors when reading stdin ([#6708] by [@andersk] and [@lydell]) + +If you had an error in your `.prettierrc` Prettier used to crash when formatting stdin. Such errors are now handled properly. + +``` +# Prettier stable +$ prettier --parser babel < test.js +(node:21531) UnhandledPromiseRejectionWarning: Error: Invalid printWidth value. Expected an integer, but received "nope". + at _loop (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:7887:63) + at Normalizer._applyNormalization (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:8000:13) + at applyNormalization (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:7817:49) + at Normalizer.normalize (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:7823:9) + at normalizeOptions$1 (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:8760:31) + at Object.normalizeApiOptions (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:8918:10) + at getOptionsForFile (/home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:44160:69) + at /home/lydell/forks/prettier/node_modules/prettier/bin-prettier.js:44214:22 + at process._tickCallback (internal/process/next_tick.js:68:7) +(node:21531) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) +(node:21531) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. + +# Prettier master +$ prettier --parser babel < test.js +[error] Invalid printWidth value. Expected an integer, but received "nope". +``` + [#5682]: https://github.com/prettier/prettier/pull/5682 [#6657]: https://github.com/prettier/prettier/pull/6657 [#5910]: https://github.com/prettier/prettier/pull/5910 @@ -1336,6 +1361,7 @@ const example1 = (): ({ p: string => string }) => (0: any); [#6694]: https://github.com/prettier/prettier/pull/6694 [#6717]: https://github.com/prettier/prettier/pull/6717 [#6728]: https://github.com/prettier/prettier/pull/6728 +[#6708]: https://github.com/prettier/prettier/pull/6708 [@brainkim]: https://github.com/brainkim [@duailibe]: https://github.com/duailibe [@gavinjoyce]: https://github.com/gavinjoyce @@ -1354,3 +1380,5 @@ const example1 = (): ({ p: string => string }) => (0: any); [@kaicataldo]: https://github.com/kaicataldo [@cryrivers]: https://github.com/Cryrivers [@voithos]: https://github.com/voithos +[@andersk]: https://github.com/andersk +[@lydell]: https://github.com/lydell diff --git a/package.json b/package.json index e9fbd11b..a83c5d82 100644 --- a/package.json +++ b/package.json @@ -109,6 +109,7 @@ "shelljs": "0.8.3", "snapshot-diff": "0.4.0", "strip-ansi": "5.2.0", + "synchronous-promise": "2.0.10", "tempy": "0.2.1", "terser-webpack-plugin": "2.1.3", "webpack": "4.41.2" diff --git a/src/cli/util.js b/src/cli/util.js index 44baae1a..1fa3c565 100644 --- a/src/cli/util.js +++ b/src/cli/util.js @@ -72,23 +72,21 @@ function handleError(context, filename, error) { } const isParseError = Boolean(error && error.loc); - const isValidationError = /Validation Error/.test(error && error.message); + const isValidationError = /^Invalid \S+ value\./.test(error && error.message); - // For parse errors and validation errors, we only want to show the error - // message formatted in a nice way. `String(error)` takes care of that. Other - // (unexpected) errors are passed as-is as a separate argument to - // `console.error`. That includes the stack trace (if any), and shows a nice - // `util.inspect` of throws things that aren't `Error` objects. (The Flow - // parser has mistakenly thrown arrays sometimes.) if (isParseError) { + // `invalid.js: SyntaxError: Unexpected token (1:1)`. context.logger.error(`${filename}: ${String(error)}`); } else if (isValidationError || error instanceof errors.ConfigError) { - context.logger.error(String(error)); + // `Invalid printWidth value. Expected an integer, but received 0.5.` + context.logger.error(error.message); // If validation fails for one file, it will fail for all of them. process.exit(1); } else if (error instanceof errors.DebugError) { + // `invalid.js: Some debug error message` context.logger.error(`${filename}: ${error.message}`); } else { + // `invalid.js: Error: Some unexpected error\n[stack trace]` context.logger.error(filename + ": " + (error.stack || error)); } @@ -368,24 +366,25 @@ function formatStdin(context) { const ignorer = createIgnorerFromContextOrDie(context); const relativeFilepath = path.relative(process.cwd(), filepath); - thirdParty.getStream(process.stdin).then(input => { - if (relativeFilepath && ignorer.filter([relativeFilepath]).length === 0) { - writeOutput(context, { formatted: input }); - return; - } + thirdParty + .getStream(process.stdin) + .then(input => { + if (relativeFilepath && ignorer.filter([relativeFilepath]).length === 0) { + writeOutput(context, { formatted: input }); + return; + } - const options = getOptionsForFile(context, filepath); + const options = getOptionsForFile(context, filepath); - try { if (listDifferent(context, input, options, "(stdin)")) { return; } writeOutput(context, format(context, input, options), options); - } catch (error) { + }) + .catch(error => { handleError(context, relativeFilepath || "stdin", error); - } - }); + }); } function createIgnorerFromContextOrDie(context) { @@ -458,7 +457,6 @@ function formatFiles(context) { }); if (isTTY()) { - // Don't use `console.log` here since we need to replace this line. context.logger.log(filename, { newline: false }); } diff --git a/tests_integration/runPrettier.js b/tests_integration/runPrettier.js index 27226065..b258d093 100644 --- a/tests_integration/runPrettier.js +++ b/tests_integration/runPrettier.js @@ -3,6 +3,7 @@ const fs = require("fs"); const path = require("path"); const stripAnsi = require("strip-ansi"); +const SynchronousPromise = require("synchronous-promise").SynchronousPromise; const isProduction = process.env.NODE_ENV === "production"; const prettierRootDir = isProduction ? process.env.PRETTIER_DIR : "../"; @@ -82,9 +83,9 @@ function runPrettier(dir, args, options) { // We cannot use `jest.setMock("get-stream", impl)` here, because in the // production build everything is bundled into one file so there is no // "get-stream" module to mock. - jest.spyOn(require(thirdParty), "getStream").mockImplementation(() => ({ - then: handler => handler(options.input || "") - })); + jest + .spyOn(require(thirdParty), "getStream") + .mockImplementation(() => SynchronousPromise.resolve(options.input || "")); jest .spyOn(require(thirdParty), "isCI") .mockImplementation(() => process.env.CI); diff --git a/yarn.lock b/yarn.lock index 2b818992..b6044f5f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6965,6 +6965,11 @@ symbol-tree@^3.2.2: version "3.2.2" resolved "https://registry.yarnpkg.com/symbol-tree/-/symbol-tree-3.2.2.tgz#ae27db38f660a7ae2e1c3b7d1bc290819b8519e6" +synchronous-promise@2.0.10: + version "2.0.10" + resolved "https://registry.yarnpkg.com/synchronous-promise/-/synchronous-promise-2.0.10.tgz#e64c6fd3afd25f423963353043f4a68ebd397fd8" + integrity sha512-6PC+JRGmNjiG3kJ56ZMNWDPL8hjyghF5cMXIFOKg+NiwwEZZIvxTWd0pinWKyD227odg9ygF8xVhhz7gb8Uq7A== + table@^5.2.3: version "5.4.0" resolved "https://registry.yarnpkg.com/table/-/table-5.4.0.tgz#d772a3216e68829920a41a32c18eda286c95d780"