fix(migrations): improve error handling for loading migrations (#52)

Author: eseliger

.gitignore

@@ -5,4 +5,4 @@ typedoc/
 .vscode/
 coverage/
 .nyc_output/
-migrations/
+./migrations/

package.json

@@ -36,6 +36,8 @@
     "yargs": "^5.0.0"
   },
   "devDependencies": {
+    "chai": "^4.1.2",
+    "chai-as-promised": "^7.1.1",
     "cz-conventional-changelog": "^2.0.0",
     "del": "^2.2.2",
     "husky": "^0.14.3",

src/migration.ts

@@ -7,6 +7,12 @@ import * as fs from 'mz/fs';
 
 export type TaskType = 'up' | 'down';
 
+export class MigrationLoadError extends Error {
+    constructor(public migration: Migration, public migrationDir: string, public error: any) {
+        super('Error loading migration file ' + migrationDir + sep + chalk.bold(migration.name) + '.js:\n' + error);
+    }
+}
+
 export class FirstDownMigrationError extends Error {
     constructor(public migration: Migration) {
         super(`The first migration cannot be a down migration (${migration.name})`);
@@ -137,11 +143,11 @@ export class Task {
                 migrationDir = config.migrationOutDir;
             }
         }
+        const path = await this.migration.getPath(migrationDir);
         try {
-            const path = await this.migration.getPath(migrationDir);
             migrationExports = require(path);
         } catch (err) {
-            throw new MigrationNotFoundError(this.migration, migrationDir);
+            throw new MigrationLoadError(this.migration, migrationDir, err);
         }
         if (typeof migrationExports[this.type] !== 'function') {
             throw new TaskTypeNotFoundError(this.migration, this.type, migrationDir);

src/test/migration.test.ts

@@ -1,9 +1,13 @@
-import * as assert from 'assert';
+import * as chai from 'chai';
+const chaiAsPromised = require('chai-as-promised');
+chai.use(chaiAsPromised);
+const assert = chai.assert;
 import * as fs from 'mz/fs';
 import * as path from 'path';
 import * as pg from 'pg';
 import {
     Migration,
+    MigrationLoadError,
     MigrationNotFoundError,
     MigrationExecutionError,
     TaskTypeNotFoundError,
@@ -129,61 +133,31 @@ describe('migration', () => {
                 const task = new Task({type: 'up', migration: new Migration({name: 'not_found'})});
                 const head = new Commit({sha1: 'HEADCOMMITSHA1'});
                 const trigger = new Commit({sha1: 'TRIGGERCOMMITSHA1'});
-                try {
-                    await task.execute(__dirname + '/migrations', adapter, head, trigger);
-                    throw new assert.AssertionError({
-                        message: 'No MigrationNotFoundError was thrown.'
-                    });
-                } catch (err) {
-                    if (!(err instanceof MigrationNotFoundError)) {
-                        throw err;
-                    }
-                }
+                await assert.isRejected(task.execute(__dirname + '/migrations', adapter, head, trigger), MigrationNotFoundError);
+            });
+            it('should throw a MigrationLoadError if the migration fails loading', async () => {
+                const task = new Task({type: 'up', migration: new Migration({name: 'error_load'})});
+                const head = new Commit({sha1: 'HEADCOMMITSHA1'});
+                const trigger = new Commit({sha1: 'TRIGGERCOMMITSHA1'});
+                await assert.isRejected(task.execute(__dirname + '/migrations', adapter, head, trigger), MigrationLoadError);
             });
             it('should throw a TaskTypeNotFoundError if the migration has no up or down function', async () => {
                 const task = new Task({type: 'up', migration: new Migration({name: 'no_up'})});
                 const head = new Commit({sha1: 'HEADCOMMITSHA1'});
                 const trigger = new Commit({sha1: 'TRIGGERCOMMITSHA1'});
-                try {
-                    await task.execute(__dirname + '/migrations', adapter, head, trigger);
-                    throw new assert.AssertionError({
-                        message: 'No TaskTypeNotFoundError was thrown.'
-                    });
-                } catch (err) {
-                    if (!(err instanceof TaskTypeNotFoundError)) {
-                        throw err;
-                    }
-                }
+                await assert.isRejected(task.execute(__dirname + '/migrations', adapter, head, trigger), TaskTypeNotFoundError);
             });
             it('should throw a MigrationExecutionError when the migration returns a rejected promise', async () => {
                 const task = new Task({type: 'up', migration: new Migration({name: 'error_async'})});
                 const head = new Commit({sha1: 'HEADCOMMITSHA1'});
                 const trigger = new Commit({sha1: 'TRIGGERCOMMITSHA1'});
-                try {
-                    await task.execute(__dirname + '/migrations', adapter, head, trigger);
-                    throw new assert.AssertionError({
-                        message: 'No MigrationExecutionError was thrown.'
-                    });
-                } catch (err) {
-                    if (!(err instanceof MigrationExecutionError)) {
-                        throw err;
-                    }
-                }
+                await assert.isRejected(task.execute(__dirname + '/migrations', adapter, head, trigger), MigrationExecutionError);
             });
             it('should throw a MigrationExecutionError when the migration throws sync', async () => {
                 const task = new Task({type: 'up', migration: new Migration({name: 'error_sync'})});
                 const head = new Commit({sha1: 'HEADCOMMITSHA1'});
                 const trigger = new Commit({sha1: 'TRIGGERCOMMITSHA1'});
-                try {
-                    await task.execute(__dirname + '/migrations', adapter, head, trigger);
-                    throw new assert.AssertionError({
-                        message: 'No Migration ExecutionError was thrown.'
-                    });
-                } catch (err) {
-                    if (!(err instanceof MigrationExecutionError)) {
-                        throw err;
-                    }
-                }
+                await assert.isRejected(task.execute(__dirname + '/migrations', adapter, head, trigger), MigrationExecutionError);
             });
             it('should throw a MigrationExecutionError when the migration would crash the process', async () => {
                 const task = new Task({type: 'up', migration: new Migration({name: 'error_crash'})});
@@ -193,14 +167,7 @@ describe('migration', () => {
                 try {
                     listener = process.listeners('uncaughtException').pop();
                     process.removeListener('uncaughtException', listener);
-                    await task.execute(__dirname + '/migrations', adapter, head, trigger);
-                    throw new assert.AssertionError({
-                        message: 'No Migration ExecutionError was thrown.'
-                    });
-                } catch (err) {
-                    if (!(err instanceof MigrationExecutionError)) {
-                        throw err;
-                    }
+                    await assert.isRejected(task.execute(__dirname + '/migrations', adapter, head, trigger), MigrationExecutionError);
                 } finally {
                     process.addListener('uncaughtException', listener);
                 }
@@ -209,16 +176,7 @@ describe('migration', () => {
         describe('toString', () => {
             it('should throw if the task type is unknown', async () => {
                 const task = new Task(<any>{type: 'd'});
-                try {
-                    task.toString();
-                    throw new assert.AssertionError({
-                        message: 'Task did not throw error on toString with wrong type'
-                    });
-                } catch (err) {
-                    if (!(err instanceof UnknownTaskTypeError)) {
-                        throw err;
-                    }
-                }
+                assert.throws(() => task.toString(), UnknownTaskTypeError);
             });
         });
     });

src/test/migrations/error_load.ts

@@ -0,0 +1,2 @@
+
+throw new Error('Migration failed loading');

typings.json

@@ -17,5 +17,9 @@
   },
   "globalDevDependencies": {
     "mocha": "registry:env/mocha#2.2.5+20160926180742"
+  },
+  "devDependencies": {
+    "chai": "registry:npm/chai#3.5.0+20160723033700",
+    "chai-as-promised": "registry:npm/chai-as-promised#5.1.0+20160310030142"
   }
 }