Improve error messages for resolution failures

Change-type: patch
Signed-off-by: Cameron Diver <[email protected]>

package.json

@@ -35,18 +35,18 @@
     "@types/semver": "^6.0.0",
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
-    "ncp": "^2.0.0",
     "husky": "^1.3.1",
     "lint-staged": "^8.1.5",
     "mkdirp": "^0.5.1",
     "mocha": "^6.1.3",
+    "ncp": "^2.0.0",
     "prettier": "^1.16.4",
     "require-npm4-to-publish": "^1.0.0",
     "resin-lint": "^2.0.1",
     "rimraf": "^2.6.3",
     "tar-utils": "^2.0.0",
     "tslint": "^5.15.0",
-    "typescript": "^3.4.3"
+    "typescript": "^3.6.2"
   },
   "dependencies": {
     "@types/tar-stream": "^1.6.0",

src/index.ts

@@ -26,7 +26,9 @@ import { Resolver } from './resolver';
 // Import some default resolvers
 import ArchDockerfileResolver from './resolvers/archDockerfile';
 import DockerfileResolver from './resolvers/dockerfile';
-import DockerfileTemplateResolver from './resolvers/dockerfileTemplate';
+import DockerfileTemplateResolver, {
+	DockerfileTemplateVariableError,
+} from './resolvers/dockerfileTemplate';
 import NodeResolver from './resolvers/nodeResolver';
 import { parsePosixPath } from './utils';
 
@@ -36,6 +38,7 @@ export {
 	Bundle,
 	DockerfileResolver,
 	DockerfileTemplateResolver,
+	DockerfileTemplateVariableError,
 	FileInfo,
 	Resolver,
 };
@@ -169,8 +172,8 @@ async function resolveTarStreamOnFinish(
 			'error',
 			new Error(
 				dockerfile
-					? `Specified dockerfile could not be resolved: ${dockerfile}`
-					: 'Resolution could not be performed',
+					? `Specified file not found or is invalid: ${dockerfile}`
+					: 'Could not find a Dockerfile for this service',
 			),
 		);
 		return;

src/resolvers/archDockerfile.ts

@@ -20,6 +20,7 @@ import * as DockerfileTemplate from 'dockerfile-template';
 
 import { Bundle, FileInfo, Resolver } from '../resolver';
 import { ParsedPathPlus, removeExtension } from '../utils';
+import { DockerfileTemplateVariableError } from './dockerfileTemplate';
 
 // Internal tuple to pass files and their extensions around
 // the class
@@ -104,10 +105,14 @@ export class ArchDockerfileResolver implements Resolver {
 			BALENA_MACHINE_NAME: bundle.deviceType,
 		};
 
-		this.dockerfileContents = DockerfileTemplate.process(
-			satisfied[1].contents.toString(),
-			vars,
-		);
+		try {
+			this.dockerfileContents = DockerfileTemplate.process(
+				satisfied[1].contents.toString(),
+				vars,
+			);
+		} catch (e) {
+			throw new DockerfileTemplateVariableError(e);
+		}
 
 		return Promise.resolve([
 			{

src/resolvers/dockerfileTemplate.ts

@@ -15,10 +15,13 @@
  * limitations under the License.
  */
 import * as DockerfileTemplate from 'dockerfile-template';
+import TypedError = require('typed-error');
 
 import { Bundle, FileInfo, Resolver } from '../resolver';
 import { ParsedPathPlus, removeExtension } from '../utils';
 
+export class DockerfileTemplateVariableError extends TypedError {}
+
 export class DockerfileTemplateResolver implements Resolver {
 	public priority = 2;
 	public name = 'Dockerfile.template';
@@ -77,10 +80,14 @@ export class DockerfileTemplateResolver implements Resolver {
 			BALENA_MACHINE_NAME: bundle.deviceType,
 		};
 
-		this.dockerfileContents = DockerfileTemplate.process(
-			this.templateContent.toString(),
-			vars,
-		);
+		try {
+			this.dockerfileContents = DockerfileTemplate.process(
+				this.templateContent.toString(),
+				vars,
+			);
+		} catch (e) {
+			throw new DockerfileTemplateVariableError(e);
+		}
 
 		return new Promise<FileInfo[]>(resolve => {
 			// FIXME: submit a PR to DockerfileTemplate to take Buffers as an input

test/tests.ts

@@ -17,6 +17,7 @@
 import { expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
 import * as fs from 'fs';
+import isString = require('lodash/isString');
 import { Readable } from 'stream';
 import * as tar from 'tar-stream';
 import * as TarUtils from 'tar-utils';
@@ -84,7 +85,7 @@ function getPromiseForEvents(
 	events: { [event: string]: (eventArg: Error | string) => void },
 	rejectOnError = true,
 	resolveOnEnd = false,
-): [Promise<object>, Resolve.ResolveListeners] {
+): [Promise<unknown>, Resolve.ResolveListeners] {
 	const listeners: Resolve.ResolveListeners = {};
 	const resolvePromise = new Promise((resolve, reject) => {
 		listeners['error'] = [rejectOnError ? reject : resolve];
@@ -281,6 +282,11 @@ describe('Resolvers', () => {
 				end: () => {
 					throw new Error('No error thrown for incorrect template variables');
 				},
+				error: err => {
+					expect(isString(err) ? err : err.message).to.equal(
+						'RESIN_DEVICE_TYPE is not defined',
+					);
+				},
 			},
 			false,
 		);
@@ -289,7 +295,7 @@ describe('Resolvers', () => {
 		return resolvePromise;
 	});
 
-	it('should reject a `nodeJS project with no engines entry', async function() {
+	it('should reject a nodeJS project with no engines entry', async function() {
 		let errorMessage: string;
 		try {
 			await testResolveInput({
@@ -306,7 +312,7 @@ describe('Resolvers', () => {
 		);
 	});
 
-	it('should resolve a nodeJS project', function() {
+	it.skip('should resolve a nodeJS project', function() {
 		this.timeout(3600000);
 		const deviceType = 'raspberrypi3';
 		return testResolveInput({
@@ -447,7 +453,7 @@ describe('Specifying dockerfiles', () => {
 			errorMessage = err.message;
 		}
 		expect(errorMessage).to.equal(
-			'Specified dockerfile could not be resolved: InexistentDockerfile',
+			'Specified file not found or is invalid: InexistentDockerfile',
 		);
 	});
 
@@ -464,6 +470,8 @@ describe('Specifying dockerfiles', () => {
 		} catch (err) {
 			errorMessage = err.message;
 		}
-		expect(errorMessage).to.equal('Resolution could not be performed');
+		expect(errorMessage).to.equal(
+			'Could not find a Dockerfile for this service',
+		);
 	});
 });

tslint.json

@@ -1,26 +1,27 @@
 {
-		"extends": "tslint:recommended",
-		"rules": {
-				"indent": [ true, "tabs" ],
-				"jsdoc-format": true,
-				"whitespace": [
-						false,
-						"check-type"
-				],
-				"object-literal-sort-keys": false,
-				"only-arrow-functions": false,
-				"quotemark": [true, "single", "avoid-escape"],
-				"no-var-requires": true,
-				"no-console": false,
-				"no-string-literal": false,
-				"arrow-parens": false,
-				"interface-name": false,
-				"variable-name": [
-						true,
-						"ban-keywords",
-						"check-format",
-						"allow-leading-underscore",
-						"allow-pascal-case"
-				]
-		}
+	"extends": "tslint:recommended",
+	"rules": {
+		"indent": [ true, "tabs" ],
+		"jsdoc-format": true,
+		"whitespace": [
+			false,
+			"check-type"
+		],
+		"object-literal-sort-keys": false,
+		"only-arrow-functions": false,
+		"quotemark": [true, "single", "avoid-escape"],
+		"no-var-requires": true,
+		"arrow-parens": false,
+		"max-classes-per-file": false,
+		"no-console": false,
+		"no-string-literal": false,
+		"interface-name": false,
+		"variable-name": [
+			true,
+			"ban-keywords",
+			"check-format",
+			"allow-leading-underscore",
+			"allow-pascal-case"
+		]
+	}
 }