New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

knex 0.8.5 migration failures getting ignored or misreported #835

Closed
jurko-gospodnetic opened this Issue May 19, 2015 · 1 comment

Comments

Projects
None yet
1 participant
@jurko-gospodnetic
Collaborator

jurko-gospodnetic commented May 19, 2015

When a knex 0.8.5 migration step fails, its Migrator._runBatch() function seems to catch the reported failure information, report it to the user as a warning message and then simply not pass it on.

I think that instead of doing this:

    }).catch(function(error) {
      helpers.warn('migrations failed with error: ' + error.message)
    });

Migrator._runBatch() should do this:

    }).catch(function(error) {
      helpers.warn('migrations failed with error: ' + error.message)
      throw error;
    });

Or it might be even better to not force an end-user warning on your caller at all and let them deal with the failure however they want. For example, we have an automated test suite that makes sure all of our data migration scripts do their work successfully, and we'd rather not have any user warning displayed during those tests, but we really want the tests to fail in case of such errors.

In some cases the current 'returning undefined' implementation will cause the error to be completely ignored (happens in our tests), while in others it will cause random JavaScript errors, e.g. BlueBird promise function complaining about expecting to receive an array and receiving a single object instead (seen happening when running the actual database migration in our development environment).

@tgriesser tgriesser closed this in 234a26e May 20, 2015

@jurko-gospodnetic

This comment has been minimized.

Collaborator

jurko-gospodnetic commented May 20, 2015

Yay! +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment