Use verror to wrap migration errors (#416)

* Use verror to wrap migration errors

* Use star import

* Add Errors section to readme

* Capitalise error messages

README.md

@@ -532,6 +532,26 @@ The [`FileLocker` class](./src/file-locker.ts) uses `beforeAll` and `afterAll` t
 
 All events are type-safe, so IDEs will prevent typos and supply strong types for the event payloads.
 
+### Errors
+
+When a migration throws an error, it will be wrapped in a `MigrationError` which captures the migration metadata (name, path etc.) as well as the original error message, and _will be rethrown_. In most cases, this is expected behaviour, and doesn't require any special handling beyond standard error logging setups.
+
+If you expect failures and want to try to recover from them, you will need to try-catch the call to `umzug.up()`. You can access the original error from the `.cause` property if necessary:
+
+```js
+try {
+  await umzug.up();
+} catch (e) {
+  if (e instanceof MigrationError) {
+    const original = e.cause;
+    // do something with the original error here
+  }
+  throw e;
+}
+```
+
+Under the hood, [verror](https://npmjs.com/package/verror) is used to wrap errors.
+
 ### CLI
 
 🚧🚧🚧 The CLI is new to Umzug v3 beta and is not yet stable. Feedback on it is welcome in [discussions](https://github.com/sequelize/umzug/discussions) 🚧🚧🚧

package.json

@@ -15,10 +15,12 @@
 	],
 	"dependencies": {
 		"@rushstack/ts-command-line": "~4.7.7",
+		"@types/verror": "^1.10.4",
 		"emittery": "~0.7.2",
 		"fs-jetpack": "~4.0.0",
 		"glob": "~7.1.6",
-		"type-fest": "~0.19.0"
+		"type-fest": "~0.19.0",
+		"verror": "^1.10.0"
 	},
 	"devDependencies": {
 		"@types/jest": "26.0.16",

src/umzug.ts

@@ -6,6 +6,7 @@ import * as templates from './templates';
 import * as glob from 'glob';
 import { UmzugCLI } from './cli';
 import * as emittery from 'emittery';
+import * as VError from 'verror';
 import {
 	MigrateDownOptions,
 	MigrateUpOptions,
@@ -21,6 +22,45 @@ import {
 
 const globAsync = promisify(glob);
 
+interface MigrationErrorParams extends MigrationParams<unknown> {
+	direction: 'up' | 'down';
+}
+
+export class Rethrowable extends VError {
+	static wrap(throwable: unknown): VError {
+		if (throwable instanceof VError) {
+			return throwable;
+		}
+
+		if (throwable instanceof Error) {
+			return new VError(throwable, 'Original error');
+		}
+
+		return new VError(
+			{
+				info: { original: throwable },
+			},
+			`Non-error value thrown. See info for full props: %s`,
+			throwable
+		);
+	}
+}
+
+export class MigrationError extends VError {
+	constructor(migration: MigrationErrorParams, original: unknown) {
+		super(
+			{
+				cause: Rethrowable.wrap(original),
+				name: 'MigrationError',
+				info: migration,
+			},
+			'Migration %s (%s) failed',
+			migration.name,
+			migration.direction
+		);
+	}
+}
+
 export class Umzug<Ctx = unknown> extends emittery.Typed<UmzugEvents<Ctx>> {
 	private readonly storage: UmzugStorage<Ctx>;
 	/** @internal */
@@ -99,7 +139,7 @@ export class Umzug<Ctx = unknown> extends emittery.Typed<UmzugEvents<Ctx>> {
 			try {
 				return require(filepath);
 			} catch (e: unknown) {
-				if (e instanceof Error && filepath.endsWith('.ts')) {
+				if (e instanceof SyntaxError && filepath.endsWith('.ts')) {
 					e.message += '\n\n' + languageSpecificHelp['.ts'];
 				}
 
@@ -237,7 +277,11 @@ export class Umzug<Ctx = unknown> extends emittery.Typed<UmzugEvents<Ctx>> {
 				this.logging({ event: 'migrating', name: m.name });
 				await this.emit('migrating', params);
 
-				await m.up(params);
+				try {
+					await m.up(params);
+				} catch (e: unknown) {
+					throw new MigrationError({ direction: 'up', ...params }, e);
+				}
 
 				await this.storage.logMigration(m.name, params);
 
@@ -293,7 +337,11 @@ export class Umzug<Ctx = unknown> extends emittery.Typed<UmzugEvents<Ctx>> {
 				this.logging({ event: 'reverting', name: m.name });
 				await this.emit('reverting', params);
 
-				await m.down?.(params);
+				try {
+					await m.down?.(params);
+				} catch (e: unknown) {
+					throw new MigrationError({ direction: 'down', ...params }, e);
+				}
 
 				await this.storage.unlogMigration(m.name, params);
 

test/umzug.test.ts

@@ -2,6 +2,7 @@ import { memoryStorage, RerunBehavior, Umzug } from '../src';
 import * as path from 'path';
 import { fsSyncer } from 'fs-syncer';
 import { expectTypeOf } from 'expect-type';
+import * as VError from 'verror';
 
 jest.mock('../src/storage', () => {
 	const storage = jest.requireActual('../src/storage');
@@ -371,10 +372,105 @@ describe('alternate migration inputs', () => {
 		expect(spy).toHaveBeenNthCalledWith(1, 'migration1-up');
 	});
 
+	test('errors are wrapped helpfully', async () => {
+		// Raw errors usually won't tell you which migration threw. This test ensures umzug adds that information.
+		const umzug = new Umzug({
+			migrations: [
+				{
+					name: 'm1',
+					up: async () => {},
+					down: async () => Promise.reject(new Error('Some cryptic failure')),
+				},
+				{
+					name: 'm2',
+					up: async () => Promise.reject(new Error('Some cryptic failure')),
+					down: async () => {},
+				},
+			],
+			logger: undefined,
+		});
+
+		await expect(umzug.up()).rejects.toThrowErrorMatchingInlineSnapshot(
+			`"Migration m2 (up) failed: Original error: Some cryptic failure"`
+		);
+
+		await expect(umzug.down()).rejects.toThrowErrorMatchingInlineSnapshot(
+			`"Migration m1 (down) failed: Original error: Some cryptic failure"`
+		);
+	});
+
+	test('Error causes are propagated properly', async () => {
+		const umzug = new Umzug({
+			migrations: [
+				{
+					name: 'm1',
+					up: async () => {},
+					down: async () => Promise.reject(new VError({ info: { extra: 'detail' } }, 'Some cryptic failure')),
+				},
+				{
+					name: 'm2',
+					up: async () => Promise.reject(new VError({ info: { extra: 'detail' } }, 'Some cryptic failure')),
+					down: async () => {},
+				},
+			],
+			logger: undefined,
+		});
+
+		await expect(umzug.up()).rejects.toThrowErrorMatchingInlineSnapshot(
+			`"Migration m2 (up) failed: Some cryptic failure"`
+		);
+
+		// slightly weird format verror uses, not worth validating much more than that the `cause` is captured
+		await expect(umzug.up()).rejects.toMatchObject({
+			jse_cause: {
+				jse_info: { extra: 'detail' },
+			},
+		});
+
+		await expect(umzug.down()).rejects.toThrowErrorMatchingInlineSnapshot(
+			`"Migration m1 (down) failed: Some cryptic failure"`
+		);
+
+		await expect(umzug.down()).rejects.toMatchObject({
+			jse_cause: {
+				jse_info: { extra: 'detail' },
+			},
+		});
+	});
+
+	test('non-error throwables are wrapped helpfully', async () => {
+		// Migration errors usually won't tell you which migration threw. This test ensures umzug adds that information.
+		const umzug = new Umzug({
+			migrations: [
+				{
+					name: 'm1',
+					up: async () => {},
+					// eslint-disable-next-line prefer-promise-reject-errors
+					down: async () => Promise.reject('Some cryptic failure'),
+				},
+				{
+					name: 'm2',
+					// eslint-disable-next-line prefer-promise-reject-errors
+					up: async () => Promise.reject('Some cryptic failure'),
+					down: async () => {},
+				},
+			],
+			logger: undefined,
+		});
+
+		await expect(umzug.up()).rejects.toThrowErrorMatchingInlineSnapshot(
+			`"Migration m2 (up) failed: Non-error value thrown. See info for full props: Some cryptic failure"`
+		);
+
+		await expect(umzug.down()).rejects.toThrowErrorMatchingInlineSnapshot(
+			`"Migration m1 (down) failed: Non-error value thrown. See info for full props: Some cryptic failure"`
+		);
+	});
+
 	test('typescript migration files', async () => {
 		const syncer = fsSyncer(path.join(__dirname, 'generated/umzug/typescript'), {
 			'm1.ts': `export const up = () => {}; export const down = () => {}`,
-			'm2.ts': `throw Error('Error to simulate typescript modules not being registered')`,
+			'm2.ts': `throw SyntaxError('Fake syntax error to simulate typescript modules not being registered')`,
 		});
 		syncer.sync();
 
@@ -392,7 +488,7 @@ describe('alternate migration inputs', () => {
 		expect([names(await umzug.pending()), names(await umzug.executed())]).toEqual([['m2.ts'], ['m1.ts']]);
 
 		await expect(umzug.up()).rejects.toThrowErrorMatchingInlineSnapshot(`
-			"Error to simulate typescript modules not being registered
+			"Migration m2.ts (up) failed: Original error: Fake syntax error to simulate typescript modules not being registered
 
 			TypeScript files can be required by adding \`ts-node\` as a dependency and calling \`require('ts-node/register')\` at the program entrypoint before running migrations."
 		`);