cli: Handle errors when reading stdin (#6708)

Co-authored-by: Simon Lydell <simon.lydell@gmail.com>

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

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"

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 });
     }
 

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);

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"