Harden 'remote-build' error handling (balena push)

Change-type: patch
Signed-off-by: Paulo Castro <paulo@balena.io>

lib/actions/push.ts

@@ -194,11 +194,11 @@ export const push: CommandDefinition<
 		switch (buildTarget) {
 			case BuildTarget.Cloud:
 				const app = appOrDevice;
-				Bluebird.join(
+				await Bluebird.join(
 					sdk.auth.getToken(),
 					sdk.settings.get('balenaUrl'),
 					getAppOwner(sdk, app),
-					(token, baseUrl, owner) => {
+					async (token, baseUrl, owner) => {
 						const opts = {
 							emulated: options.emulated,
 							nocache: options.nocache,
@@ -214,14 +214,14 @@ export const push: CommandDefinition<
 							opts,
 						};
 
-						return remote.startRemoteBuild(args);
+						return await remote.startRemoteBuild(args);
 					},
 				).nodeify(done);
 				break;
 			case BuildTarget.Device:
 				const device = appOrDevice;
 				// TODO: Support passing a different port
-				Bluebird.resolve(
+				await Bluebird.resolve(
 					deviceDeploy.deployToDevice({
 						source,
 						deviceHost: device,

lib/utils/remote-build.ts

@@ -13,15 +13,17 @@ 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 * as Bluebird from 'bluebird';
 import * as JSONStream from 'JSONStream';
+import * as readline from 'readline';
 import * as request from 'request';
-import { BalenaSDK } from 'balena-sdk';
 import * as Stream from 'stream';
+import { BalenaSDK } from 'balena-sdk';
 import { Pack } from 'tar-stream';
+import { RegistrySecrets } from 'resin-multibuild';
 import { TypedError } from 'typed-error';
 
-import { RegistrySecrets } from 'resin-multibuild';
+import { exitWithExpectedError } from '../utils/patterns';
 import { tarDirectory } from './compose';
 
 const DEBUG_MODE = !!process.env.DEBUG;
@@ -85,18 +87,16 @@ async function getBuilderEndpoint(
 }
 
 export async function startRemoteBuild(build: RemoteBuild): Promise<void> {
-	const Bluebird = await import('bluebird');
+	const stream = await getRemoteBuildStream(build);
-
-	const stream = await getRequestStream(build);
 
 	// Special windows handling (win64 also reports win32)
 	if (process.platform === 'win32') {
-		const readline = (await import('readline')).createInterface({
+		const rl = readline.createInterface({
 			input: process.stdin,
 			output: process.stdout,
 		});
 
-		readline.on('SIGINT', () => process.emit('SIGINT'));
+		rl.on('SIGINT', () => process.emit('SIGINT'));
 	}
 
 	return new Bluebird((resolve, reject) => {
@@ -126,13 +126,11 @@ export async function startRemoteBuild(build: RemoteBuild): Promise<void> {
 	});
 }
 
-async function handleBuilderMetadata(obj: BuilderMessage, build: RemoteBuild) {
+function handleBuilderMetadata(obj: BuilderMessage, build: RemoteBuild) {
-	const { stripIndent } = await import('common-tags');
+	const { stripIndent } = require('common-tags');
 
 	switch (obj.resource) {
 		case 'cursor':
-			const readline = await import('readline');
-
 			if (obj.value == null) {
 				return;
 			}
@@ -177,8 +175,8 @@ async function handleBuilderMetadata(obj: BuilderMessage, build: RemoteBuild) {
 
 function getBuilderMessageHandler(
 	build: RemoteBuild,
-): (obj: BuilderMessage) => Promise<void> {
+): (obj: BuilderMessage) => void {
-	return async (obj: BuilderMessage) => {
+	return (obj: BuilderMessage) => {
 		if (DEBUG_MODE) {
 			console.log(`[debug] handling message: ${JSON.stringify(obj)}`);
 		}
@@ -186,7 +184,6 @@ function getBuilderMessageHandler(
 			return handleBuilderMetadata(obj, build);
 		}
 		if (obj.message) {
-			const readline = await import('readline');
 			readline.clearLine(process.stdout, 0);
 
 			const message = obj.message.replace(TRIM_REGEX, '');
@@ -216,72 +213,119 @@ async function cancelBuildIfNecessary(build: RemoteBuild): Promise<void> {
 }
 
 /**
- * Return a callback function that takes a tar-stream Pack object as argument
+ * Call tarDirectory() with a suitable callback to insert registry secrets in
- * and uses it to add the '.balena/registry-secrets.json' metadata file that
+ * the tar stream, and return the stream.
- * contains usernames and passwords to private docker registries. The builder
- * will remove the file from the tar stream and use the secrets to pull base
- * images from users' private registries.
- * @param registrySecrets JS object containing registry usernames and passwords
- * @returns A callback function, or undefined if registrySecrets is empty
  */
-function getTarStreamCallbackForRegistrySecrets(
+async function getTarStream(build: RemoteBuild): Promise<Stream.Readable> {
-	registrySecrets: RegistrySecrets,
+	const path = await import('path');
-): ((pack: Pack) => void) | undefined {
+	const visuals = await import('resin-cli-visuals');
-	if (Object.keys(registrySecrets).length > 0) {
+	const tarSpinner = new visuals.Spinner('Packaging the project source...');
-		return (pack: Pack) => {
+	const preFinalizeCallback = (pack: Pack) => {
-			pack.entry(
+		pack.entry(
-				{ name: '.balena/registry-secrets.json' },
+			{ name: '.balena/registry-secrets.json' },
-				JSON.stringify(registrySecrets),
+			JSON.stringify(build.opts.registrySecrets),
-			);
+		);
-		};
+	};
+
+	try {
+		tarSpinner.start();
+		return await tarDirectory(
+			path.resolve(build.source),
+			Object.keys(build.opts.registrySecrets).length > 0
+				? preFinalizeCallback
+				: undefined,
+		);
+	} finally {
+		tarSpinner.stop();
 	}
 }
 
-async function getRequestStream(build: RemoteBuild): Promise<Stream.Duplex> {
+/**
-	const path = await import('path');
+ * Initiate a POST HTTP request to the remote builder and add some event
-	const visuals = await import('resin-cli-visuals');
+ * listeners.
-	const zlib = await import('zlib');
+ *
-
+ * ¡! Note: this function must be synchronous because of a bug in the `request`
-	const tarSpinner = new visuals.Spinner('Packaging the project source...');
+ *    library that requires the following two steps to take place in the same
-	tarSpinner.start();
+ *    iteration of Node's event loop: (1) adding a listener for the 'response'
-	// Tar the directory so that we can send it to the builder
+ *    event and (2) calling request.pipe():
-	const tarStream = await tarDirectory(
+ *    https://github.com/request/request/issues/887
-		path.resolve(build.source),
+ */
-		getTarStreamCallbackForRegistrySecrets(build.opts.registrySecrets),
+function createRemoteBuildRequest(
-	);
+	build: RemoteBuild,
-	tarSpinner.stop();
+	tarStream: Stream.Readable,
-
+	builderUrl: string,
-	const url = await getBuilderEndpoint(
+	onError: (error: Error) => void,
-		build.baseUrl,
+): request.Request {
-		build.owner,
+	const zlib = require('zlib');
-		build.app,
-		build.opts,
-	);
-
 	if (DEBUG_MODE) {
-		console.log(`[debug] Connecting to builder at ${url}`);
+		console.log(`[debug] Connecting to builder at ${builderUrl}`);
 	}
-	const post = request.post({
+	return request
-		url,
+		.post({
-		auth: {
+			url: builderUrl,
-			bearer: build.auth,
+			auth: { bearer: build.auth },
-		},
+			headers: { 'Content-Encoding': 'gzip' },
-		headers: {
+			body: tarStream.pipe(zlib.createGzip({ level: 6 })),
-			'Content-Encoding': 'gzip',
+		})
-		},
+		.on('error', onError)
-		body: tarStream.pipe(
+		.once('response', (response: request.RequestResponse) => {
-			zlib.createGzip({
+			if (response.statusCode >= 100 && response.statusCode < 400) {
-				level: 6,
+				if (DEBUG_MODE) {
-			}),
+					console.log(
-		),
+						`[debug] received HTTP ${response.statusCode} ${
-	});
+							response.statusMessage
+						}`,
+					);
+				}
+			} else {
+				let msgArr = [
+					'Remote builder responded with HTTP error:',
+					`${response.statusCode} ${response.statusMessage}`,
+				];
+				if (response.body) {
+					msgArr.push(response.body);
+				}
+				onError(new Error(msgArr.join('\n')));
+			}
+		});
+}
 
+async function getRemoteBuildStream(
+	build: RemoteBuild,
+): Promise<NodeJS.ReadWriteStream> {
+	const tarStream = await getTarStream(build);
+	const visuals = await import('resin-cli-visuals');
 	const uploadSpinner = new visuals.Spinner(
 		'Uploading source package to balena cloud',
 	);
-	uploadSpinner.start();
+	const exitOnError = (error: Error): never => {
+		uploadSpinner.stop();
+		return exitWithExpectedError(error);
+	};
 
-	const parseStream = post.pipe(JSONStream.parse('*'));
+	try {
-	parseStream.on('data', () => uploadSpinner.stop());
+		uploadSpinner.start();
-	return parseStream as Stream.Duplex;
+		const builderUrl = await getBuilderEndpoint(
+			build.baseUrl,
+			build.owner,
+			build.app,
+			build.opts,
+		);
+		const buildRequest = createRemoteBuildRequest(
+			build,
+			tarStream,
+			builderUrl,
+			exitOnError,
+		);
+		return buildRequest.pipe(
+			JSONStream.parse('*')
+				.once('close', () => uploadSpinner.stop())
+				.once('data', () => uploadSpinner.stop())
+				.once('end', () => uploadSpinner.stop())
+				.once('error', () => uploadSpinner.stop())
+				.once('finish', () => uploadSpinner.stop()),
+		);
+	} catch (error) {
+		return exitOnError(error);
+	}
 }