Skip to content
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

Fix error message bubbling up on seed error #3248

Merged
merged 3 commits into from Jun 3, 2019
Merged

Conversation

risseraka
Copy link
Contributor

@risseraka risseraka commented Jun 3, 2019

Problem: When the nth seed throws an error, all subsequent seeds add an error message on top of original message which makes it less clear which seed the error originated from.

Solution: Nest promises and catch error inside the nested seed promise.

Before:

Error: Error while executing ".../knex/seeds/9-titres-activites.js" seed: Error while executing ".../knex/seeds/8-metas-activites.js" seed: Error while e
xecuting ".../knex/seeds/7-titres.js" seed: insert into "titres_points" ("contour", "coordonnees", "description", "groupe", "id", "nom", "point"
    at Object.current.then.then.catch.originalError (.../node_modules/knex/lib/seed/Seeder.js:166:23)
Error: Error while executing ".../knex/seeds/8-metas-activites.js" seed: Error while executing ".../knex/seeds/7-titres.js" seed: insert into "titres_poi
nts" ("contour", "coordonnees", "description", "groupe", "id", "nom", "point"
    at Object.current.then.then.catch.originalError (.../node_modules/knex/lib/seed/Seeder.js:166:23)
Error: Error while executing ".../knex/seeds/7-titres.js" seed: insert into "titres_points" ("contour", "coordonnees", "description", "groupe", "id", "nom", "point" [...]
    at Object.current.then.then.catch.originalError (.../node_modules/knex/lib/seed/Seeder.js:166:23)

After:

Error: Error while executing ".../knex/seeds/7-titres.js" seed: insert into "titres_points" ("contour", "coordonnees", "description", "groupe", "id", "nom", "point" [...]
    at Object.current.then.then.catch.originalError (.../node_modules/knex/lib/seed/Seeder.js:166:23)

LMKWYT,
Cheers.

@kibertoad
Copy link
Collaborator

Thanks! Could you add unit test for that?

@risseraka
Copy link
Contributor Author

Hi Igor,

Sure, this behavior is not trivial, but I'll try to come up with something.

@risseraka
Copy link
Contributor Author

risseraka commented Jun 3, 2019

Alright, I've added a straightforward test, which fails on master but passes on this branch.

Error output on master:

Error while executing "[...]/knex/test/unit/seed/test/2-second.js" seed: Error while executing "[...]/knex/test/unit/seed/test/1-first.js" seed: throwing in first file

Error output on this branch:

Error while executing "[...]/knex/test/unit/seed/test/1-first.js" seed: throwing in first file

PS: tests seem to be quite broken on the past few pull requests, is it something to be worried about?

test/unit/seed/seeder.js Outdated Show resolved Hide resolved
@risseraka risseraka changed the title Fix bubbling up on seed error Fix error message bubbling up on seed error Jun 3, 2019
@kibertoad
Copy link
Collaborator

@risseraka There are some flaky Oracle tests, these are to be ignored, anything else - I'd need to check that out :)

@kibertoad kibertoad merged commit 9b74ccb into knex:master Jun 3, 2019
@kibertoad
Copy link
Collaborator

Thanks a lot for your contribution!

@risseraka
Copy link
Contributor Author

You're very welcome, thanks for your and every contributor's hard work.

(Impressive speed by the way. 😅)

@kibertoad
Copy link
Collaborator

My pleasure!
How urgently do you need this change released? I wasn't planning another release before 0.18 unless something serious popped up, but we can figure something out if it's something you need urgently.

@risseraka
Copy link
Contributor Author

@kibertoad No worries, there's absolutely no urgency, it was just a bit annoying to look at logs on errors, so it's mostly cosmetic, take your time to release whenever you see fit.

@kibertoad
Copy link
Collaborator

Released in 0.18.0-next1

@risseraka
Copy link
Contributor Author

Thanks for the heads up.
Hurray for @kibertoad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants