From 2307a15b105006f5704056fae0f67b15ff322595 Mon Sep 17 00:00:00 2001
From: Paulo Castro <paulo@balena.io>
Date: Thu, 2 Jul 2020 00:50:32 +0100
Subject: [PATCH] balena ssh: Refactor error handling and test cases

Connects-to: #1896
Change-type: patch
---
 lib/actions-oclif/ssh.ts |  4 ++--
 lib/utils/device/ssh.ts  |  4 ++--
 lib/utils/ssh.ts         | 29 +++++++++++++----------------
 tests/balena-api-mock.ts | 24 ++++++++++++++++++------
 4 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/lib/actions-oclif/ssh.ts b/lib/actions-oclif/ssh.ts
index 23541f28..4b4bc81e 100644
--- a/lib/actions-oclif/ssh.ts
+++ b/lib/actions-oclif/ssh.ts
@@ -128,7 +128,7 @@ export default class NoteCmd extends Command {
 		const { checkLoggedIn, getOnlineTargetUuid } = await import(
 			'../utils/patterns'
 		);
-		const { spawnSshAndExitOnError } = await import('../utils/ssh');
+		const { spawnSshAndThrowOnError } = await import('../utils/ssh');
 		const sdk = getBalenaSdk();
 
 		const proxyConfig = getProxyConfig();
@@ -248,7 +248,7 @@ export default class NoteCmd extends Command {
 			username: username!,
 		});
 
-		return spawnSshAndExitOnError(command);
+		return spawnSshAndThrowOnError(command);
 	}
 
 	async getContainerId(
diff --git a/lib/utils/device/ssh.ts b/lib/utils/device/ssh.ts
index 8cc155fd..9b4801c4 100644
--- a/lib/utils/device/ssh.ts
+++ b/lib/utils/device/ssh.ts
@@ -30,7 +30,7 @@ export async function performLocalDeviceSSH(
 	opts: DeviceSSHOpts,
 ): Promise<void> {
 	const { escapeRegExp, reduce } = await import('lodash');
-	const { spawnSshAndExitOnError } = await import('../ssh');
+	const { spawnSshAndThrowOnError } = await import('../ssh');
 	const { ExpectedError } = await import('../../errors');
 
 	let command = '';
@@ -107,7 +107,7 @@ export async function performLocalDeviceSSH(
 		command = `${deviceContainerEngineBinary} exec -i ${ttyFlag} ${containerId} ${shellCmd}`;
 	}
 
-	return spawnSshAndExitOnError([
+	return spawnSshAndThrowOnError([
 		...(opts.verbose ? ['-vvv'] : []),
 		'-t',
 		...['-p', opts.port ? opts.port.toString() : '22222'],
diff --git a/lib/utils/ssh.ts b/lib/utils/ssh.ts
index bb98a390..3976240f 100644
--- a/lib/utils/ssh.ts
+++ b/lib/utils/ssh.ts
@@ -18,6 +18,8 @@ import { spawn, StdioOptions } from 'child_process';
 import * as _ from 'lodash';
 import { TypedError } from 'typed-error';
 
+import { ExpectedError } from '../errors';
+
 export class ExecError extends TypedError {
 	public cmd: string;
 	public exitCode: number;
@@ -120,14 +122,13 @@ export const getDeviceOsRelease = _.memoize(async (deviceIp: string) =>
 /**
  * Obtain the full path for ssh using which, then spawn a child process.
  * - If the child process returns error code 0, return the function normally
- *   (do not call process.exit()).
- * - If the child process returns a non-zero error code, print a single-line
- *   warning message and call process.exit(code) with the same non-zero error
- *   code.
- * - If the child process is terminated by a process signal, print a
- *   single-line warning message and call process.exit(1).
+ *   (do not throw an error).
+ * - If the child process returns a non-zero error code, set process.exitCode
+ *   to that error code, and throw ExpectedError with a warning message.
+ * - If the child process is terminated by a process signal, set
+ *   process.exitCode = 1, and throw ExpectedError with a warning message.
  */
-export async function spawnSshAndExitOnError(
+export async function spawnSshAndThrowOnError(
 	args: string[],
 	options?: import('child_process').SpawnOptions,
 ) {
@@ -145,14 +146,10 @@ export async function spawnSshAndExitOnError(
 		// Another example, typing "exit 1" on an interactive shell causes ssh
 		// to return exit code 1. In these cases, print a short one-line warning
 		// message, and exits the CLI process with the same error code.
-		const codeMsg = exitSignal
-			? `was terminated with signal "${exitSignal}"`
-			: `exited with non-zero code "${exitCode}"`;
-		console.error(`Warning: ssh process ${codeMsg}`);
-		// TODO: avoid process.exit by refactoring CLI error handling to allow
-		// exiting with an error code and single-line warning "without a fuss"
-		// about contacting support and filing Github issues. (ExpectedError
-		// does not currently devlivers that.)
-		process.exit(exitCode || 1);
+		process.exitCode = exitCode;
+		const msg = exitSignal
+			? `Warning: ssh process was terminated with signal "${exitSignal}"`
+			: `Warning: ssh process exited with non-zero code "${exitCode}"`;
+		throw new ExpectedError(msg);
 	}
 }
diff --git a/tests/balena-api-mock.ts b/tests/balena-api-mock.ts
index 09227ffe..9d47f7e2 100644
--- a/tests/balena-api-mock.ts
+++ b/tests/balena-api-mock.ts
@@ -31,12 +31,24 @@ export class BalenaAPIMock extends NockMock {
 		super(/api\.balena-cloud\.com/);
 	}
 
-	public expectGetApplication(opts: ScopeOpts = {}) {
-		this.optGet(/^\/v5\/application($|[(?])/, opts).replyWithFile(
-			200,
-			path.join(apiResponsePath, 'application-GET-v5-expanded-app-type.json'),
-			jHeader,
-		);
+	public expectGetApplication({
+		notFound = false,
+		optional = false,
+		persist = false,
+	} = {}) {
+		const interceptor = this.optGet(/^\/v5\/application($|[(?])/, {
+			optional,
+			persist,
+		});
+		if (notFound) {
+			interceptor.reply(200, { d: [] });
+		} else {
+			interceptor.replyWithFile(
+				200,
+				path.join(apiResponsePath, 'application-GET-v5-expanded-app-type.json'),
+				jHeader,
+			);
+		}
 	}
 
 	public expectDownloadConfig(opts: ScopeOpts = {}) {