Improve NotFoundError handling

Use isNotFoundError which converts an error of the default type
`unknown` into NotFoundError if the error is an instance of NotFoundError.
Thrown errors are of type `unknown` by default so we should use methods
with type guards for better type narrowing.

Signed-off-by: Christina Ying Wang <christina@balena.io>

docker-compose.test.yml

@@ -9,7 +9,7 @@ services:
       dockerfile: Dockerfile.template
       args:
         ARCH: ${ARCH:-amd64}
-    command: ['/wait-for-it.sh', '--', '/usr/src/app/entry.sh']
+    command: [ '/wait-for-it.sh', '--', '/usr/src/app/entry.sh' ]
     # Use bridge networking for the tests
     network_mode: 'bridge'
     networks:
@@ -30,7 +30,7 @@ services:
       - ./test/data/root:/mnt/root
       - ./test/lib/wait-for-it.sh:/wait-for-it.sh
     tmpfs:
-      - /data
+      - /data # sqlite3 database
 
   dbus:
     image: balenablocks/dbus
@@ -74,7 +74,7 @@ services:
         '--',
         'npm',
         'run',
-        'test:integration',
+        'test:integration'
       ]
     depends_on:
       - balena-supervisor

package.json

@@ -20,7 +20,7 @@
     "test:env": "ARCH=$(./build-utils/detect-arch.sh) docker-compose -f docker-compose.test.yml -f docker-compose.dev.yml up --build; npm run compose:down",
     "test:compose": "ARCH=$(./build-utils/detect-arch.sh) docker-compose -f docker-compose.yml -f docker-compose.test.yml up --build --remove-orphans --exit-code-from=sut ; npm run compose:down",
     "test": "npm run lint && npm run test:build && npm run test:unit && npm run test:legacy",
-    "compose:down": "docker-compose -f docker-compose.test.yml down",
+    "compose:down": "docker-compose -f docker-compose.test.yml down && docker volume rm $(docker volume ls -f name=balena-supervisor -q)",
     "prettify": "balena-lint -e ts -e js --fix src/ test/ typings/ build-utils/ webpack.config.js",
     "release": "tsc --project tsconfig.release.json && mv build/src/* build",
     "sync": "ts-node --files sync/sync.ts",

src/compose/app.ts

@@ -18,7 +18,7 @@ import constants = require('../lib/constants');
 
 import { getStepsFromStrategy } from './update-strategies';
 
-import { InternalInconsistencyError, NotFoundError } from '../lib/errors';
+import { InternalInconsistencyError, isNotFoundError } from '../lib/errors';
 import * as config from '../config';
 import { checkTruthy, checkString } from '../lib/validation';
 import { ServiceComposeConfig, DeviceMetadata } from './types/service';
@@ -804,8 +804,8 @@ export class App {
 					let imageInfo: ImageInspectInfo | undefined;
 					try {
 						imageInfo = await imageManager.inspectByName(svc.image);
-					} catch (e: any) {
+					} catch (e: unknown) {
-						if (!NotFoundError(e)) {
+						if (!isNotFoundError(e)) {
 							throw e;
 						}
 					}

src/compose/images.ts

@@ -11,7 +11,7 @@ import { DeltaFetchOptions, FetchOptions, docker } from '../lib/docker-utils';
 import * as dockerUtils from '../lib/docker-utils';
 import {
 	DeltaStillProcessingError,
-	NotFoundError,
+	isNotFoundError,
 	StatusError,
 } from '../lib/errors';
 import * as LogTypes from '../lib/log-types';
@@ -236,8 +236,8 @@ export async function triggerFetch(
 		await markAsSupervised({ ...image, dockerImageId: img.Id });
 
 		success = true;
-	} catch (e: any) {
+	} catch (e: unknown) {
-		if (!NotFoundError(e)) {
+		if (!isNotFoundError(e)) {
 			if (!(e instanceof ImageDownloadBackoffError)) {
 				addImageFailure(image.name);
 			}
@@ -729,8 +729,8 @@ async function removeImageIfNotNeeded(image: Image): Promise<void> {
 
 		// Mark the image as removed
 		removed = true;
-	} catch (e: any) {
+	} catch (e: unknown) {
-		if (NotFoundError(e)) {
+		if (isNotFoundError(e)) {
 			removed = false;
 		} else {
 			throw e;

src/compose/network-manager.ts

@@ -3,7 +3,7 @@ import * as _ from 'lodash';
 
 import * as constants from '../lib/constants';
 import { docker } from '../lib/docker-utils';
-import { NotFoundError } from '../lib/errors';
+import { isNotFoundError } from '../lib/errors';
 import logTypes = require('../lib/log-types');
 import log from '../lib/supervisor-console';
 import { exists } from '../lib/fs-utils';
@@ -45,8 +45,8 @@ export async function create(network: Network) {
 
 		// We have a network with the same config and name
 		// already created, we can skip this
-	} catch (e: any) {
+	} catch (e: unknown) {
-		if (!NotFoundError(e)) {
+		if (!isNotFoundError(e)) {
 			logger.logSystemEvent(logTypes.createNetworkError, {
 				network: { name: network.name, appUuid: network.appUuid },
 				error: e,
@@ -120,7 +120,7 @@ export function ensureSupervisorNetwork(): Bluebird<void> {
 				});
 			}
 		})
-		.catch(NotFoundError, () => {
+		.catch(isNotFoundError, () => {
 			log.debug(`Creating ${constants.supervisorNetworkInterface} network`);
 			return Bluebird.resolve(
 				docker.createNetwork({

src/compose/service-manager.ts

@@ -15,7 +15,7 @@ import { PermissiveNumber } from '../config/types';
 import constants = require('../lib/constants');
 import {
 	InternalInconsistencyError,
-	NotFoundError,
+	isNotFoundError,
 	StatusCodeError,
 	isStatusError,
 } from '../lib/errors';
@@ -72,8 +72,8 @@ export const getAll = async (
 				service.status = vState.status;
 			}
 			return service;
-		} catch (e: any) {
+		} catch (e: unknown) {
-			if (NotFoundError(e)) {
+			if (isNotFoundError(e)) {
 				return null;
 			}
 			throw e;
@@ -206,8 +206,8 @@ export async function remove(service: Service) {
 
 	try {
 		await docker.getContainer(existingService.containerId).remove({ v: true });
-	} catch (e: any) {
+	} catch (e: unknown) {
-		if (!NotFoundError(e)) {
+		if (!isNotFoundError(e)) {
 			logger.logSystemEvent(LogTypes.removeDeadServiceError, {
 				service,
 				error: e,
@@ -227,8 +227,8 @@ async function create(service: Service) {
 			);
 		}
 		return docker.getContainer(existing.containerId);
-	} catch (e: any) {
+	} catch (e: unknown) {
-		if (!NotFoundError(e)) {
+		if (!isNotFoundError(e)) {
 			logger.logSystemEvent(LogTypes.installServiceError, {
 				service,
 				error: e,
@@ -383,8 +383,8 @@ export function listenToEvents() {
 						let service: Service | null = null;
 						try {
 							service = await getByDockerContainerId(data.id);
-						} catch (e: any) {
+						} catch (e: unknown) {
-							if (!NotFoundError(e)) {
+							if (!isNotFoundError(e)) {
 								throw e;
 							}
 						}

src/compose/volume-manager.ts

@@ -3,7 +3,7 @@ import * as Path from 'path';
 import { VolumeInspectInfo } from 'dockerode';
 
 import constants = require('../lib/constants');
-import { NotFoundError, InternalInconsistencyError } from '../lib/errors';
+import { isNotFoundError, InternalInconsistencyError } from '../lib/errors';
 import { safeRename } from '../lib/fs-utils';
 import { docker } from '../lib/docker-utils';
 import * as LogTypes from '../lib/log-types';
@@ -58,8 +58,8 @@ export async function create(volume: Volume): Promise<void> {
 		if (!volume.isEqualConfig(existing)) {
 			throw new ResourceRecreationAttemptError('volume', volume.name);
 		}
-	} catch (e: any) {
+	} catch (e: unknown) {
-		if (!NotFoundError(e)) {
+		if (!isNotFoundError(e)) {
 			logger.logSystemEvent(LogTypes.createVolumeError, {
 				volume: { name: volume.name },
 				error: e,

src/lib/errors.ts

@@ -22,16 +22,23 @@ export class StatusError extends Error {
 export const isStatusError = (x: unknown): x is StatusError =>
 	x != null && x instanceof Error && !isNaN((x as any).statusCode);
 
+export class NotFoundError extends Error {
+	public statusCode: number;
+	constructor() {
+		super();
+		this.statusCode = 404;
+	}
+}
+
+export const isNotFoundError = (e: unknown): e is NotFoundError =>
+	isStatusError(e) && e.statusCode === 404;
+
 interface CodedSysError extends Error {
 	code?: string;
 }
 
 export class DeviceNotFoundError extends TypedError {}
 
-export function NotFoundError(err: StatusCodeError): boolean {
-	return checkInt(err.statusCode) === 404;
-}
-
 export function ENOENT(err: CodedSysError): boolean {
 	return err.code === 'ENOENT';
 }

src/lib/legacy.ts

@@ -10,7 +10,7 @@ import * as applicationManager from '../compose/application-manager';
 import {
 	StatusError,
 	DatabaseParseError,
-	NotFoundError,
+	isNotFoundError,
 	InternalInconsistencyError,
 } from '../lib/errors';
 import * as constants from '../lib/constants';
@@ -145,12 +145,12 @@ export async function normaliseLegacyDatabase() {
 		const imageFromDocker = await docker
 			.getImage(service.image)
 			.inspect()
-			.catch((error) => {
+			.catch((e: unknown) => {
-				if (error instanceof NotFoundError) {
+				if (isNotFoundError(e)) {
 					return;
 				}
 
-				throw error;
+				throw e;
 			});
 		const imagesFromDatabase = await db
 			.models('image')

src/lib/migration.ts

@@ -9,7 +9,7 @@ const rimrafAsync = Bluebird.promisify(rimraf);
 import * as volumeManager from '../compose/volume-manager';
 import * as deviceState from '../device-state';
 import * as constants from '../lib/constants';
-import { BackupError, NotFoundError } from '../lib/errors';
+import { BackupError, isNotFoundError } from '../lib/errors';
 import { exec, pathExistsOnHost, mkdirp } from '../lib/fs-utils';
 import { log } from '../lib/supervisor-console';
 
@@ -67,11 +67,11 @@ export async function loadBackupFromMigration(
 					.then((volume) => {
 						return volume.remove();
 					})
-					.catch((error) => {
+					.catch((e: unknown) => {
-						if (error instanceof NotFoundError) {
+						if (isNotFoundError(e)) {
 							return;
 						}
-						throw error;
+						throw e;
 					});
 
 				await volumeManager.createFromPath(

test/lib/mocked-dockerode.ts

@@ -3,15 +3,7 @@ process.env.DOCKER_HOST = 'unix:///your/dockerode/mocks/are/not/working';
 import * as dockerode from 'dockerode';
 import { Stream } from 'stream';
 import _ = require('lodash');
-import { TypedError } from 'typed-error';
+import { NotFoundError } from '~/lib/errors';
-
-export class NotFoundError extends TypedError {
-	public statusCode: number;
-	constructor() {
-		super();
-		this.statusCode = 404;
-	}
-}
 
 const overrides: Dictionary<(...args: any[]) => Resolvable<any>> = {};