From 459aa488b801198c034eaa0fb5f3b845a428b09f Mon Sep 17 00:00:00 2001
From: Noah Allen <noahtallen@gmail.com>
Date: Wed, 12 Feb 2020 20:08:49 -0800
Subject: [PATCH] wp-env: no longer show error message twice (#20157)

* Handle errors more robustly

- Separate cases for errors we know about
- More straightforward general error case

* Update tests to handle the new error format
---
 packages/env/lib/cli.js  | 29 +++++++++++++++++++++++++----
 packages/env/test/cli.js | 21 ++++++++++++++-------
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/packages/env/lib/cli.js b/packages/env/lib/cli.js
index b589e9cce5ecf7..cfeb312db3fa3b 100644
--- a/packages/env/lib/cli.js
+++ b/packages/env/lib/cli.js
@@ -34,12 +34,33 @@ const withSpinner = ( command ) => ( ...args ) => {
 			);
 		},
 		( error ) => {
-			spinner.fail( error.message || error.err );
-			if ( ! ( error instanceof env.ValidationError ) ) {
+			if ( error instanceof env.ValidationError ) {
+				// Error is a validation error. That means the user did something wrong.
+				spinner.fail( error.message );
+				process.exit( 1 );
+			} else if (
+				'exitCode' in error &&
+				'err' in error &&
+				'out' in error
+			) {
+				// Error is a docker-compose error. That means something docker-related failed.
+				// https://github.com/PDMLab/docker-compose/blob/master/src/index.ts
+				spinner.fail( 'Error while running docker-compose command.' );
+				if ( error.out ) {
+					process.stdout.write( error.out );
+				}
+				if ( error.err ) {
+					process.stderr.write( error.err );
+				}
+				process.exit( error.exitCode );
+			} else {
+				// Error is an unknown error. That means there was a bug in our code.
+				spinner.fail( error.message );
+				// Disable reason: Using console.error() means we get a stack trace.
 				// eslint-disable-next-line no-console
-				console.error( `\n\n${ error.out || error.err }\n\n` );
+				console.error( error );
+				process.exit( 1 );
 			}
-			process.exit( error.exitCode || 1 );
 		}
 	);
 };
diff --git a/packages/env/test/cli.js b/packages/env/test/cli.js
index ef19cc816dd0b3..051a128db19310 100644
--- a/packages/env/test/cli.js
+++ b/packages/env/test/cli.js
@@ -84,8 +84,6 @@ describe( 'env cli', () => {
 		/* eslint-disable no-console */
 		env.start.mockRejectedValueOnce( {
 			message: 'failure message',
-			out: 'failure message',
-			exitCode: 2,
 		} );
 		const consoleError = console.error;
 		console.error = jest.fn();
@@ -98,28 +96,37 @@ describe( 'env cli', () => {
 
 		expect( spinner.fail ).toHaveBeenCalledWith( 'failure message' );
 		expect( console.error ).toHaveBeenCalled();
-		expect( process.exit ).toHaveBeenCalledWith( 2 );
+		expect( process.exit ).toHaveBeenCalledWith( 1 );
 		console.error = consoleError;
 		process.exit = processExit;
 		/* eslint-enable no-console */
 	} );
-	it( 'handles failed commands with errors.', async () => {
+	it( 'handles failed docker commands with errors.', async () => {
 		/* eslint-disable no-console */
-		env.start.mockRejectedValueOnce( { err: 'failure error' } );
+		env.start.mockRejectedValueOnce( {
+			err: 'failure error',
+			out: 'message',
+			exitCode: 1,
+		} );
 		const consoleError = console.error;
 		console.error = jest.fn();
 		const processExit = process.exit;
 		process.exit = jest.fn();
+		const stderr = process.stderr.write;
+		process.stderr.write = jest.fn();
 
 		cli().parse( [ 'start' ] );
 		const { spinner } = env.start.mock.calls[ 0 ][ 0 ];
 		await env.start.mock.results[ 0 ].value.catch( () => {} );
 
-		expect( spinner.fail ).toHaveBeenCalledWith( 'failure error' );
-		expect( console.error ).toHaveBeenCalled();
+		expect( spinner.fail ).toHaveBeenCalledWith(
+			'Error while running docker-compose command.'
+		);
+		expect( process.stderr.write ).toHaveBeenCalledWith( 'failure error' );
 		expect( process.exit ).toHaveBeenCalledWith( 1 );
 		console.error = consoleError;
 		process.exit = processExit;
+		process.stderr.write = stderr;
 		/* eslint-enable no-console */
 	} );
 } );