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

Conversation

Projects
None yet
2 participants
@risseraka
Copy link
Contributor

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

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

Thanks! Could you add unit test for that?

@risseraka risseraka force-pushed the risseraka:master branch from 5bd8822 to 4e4f498 Jun 3, 2019

@risseraka

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

Hi Igor,

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

@risseraka

This comment has been minimized.

Copy link
Contributor Author

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?

Show resolved Hide resolved test/unit/seed/seeder.js Outdated

@risseraka risseraka force-pushed the risseraka:master branch from a28dc51 to 812b55c Jun 3, 2019

@risseraka risseraka changed the title Fix bubbling up on seed error Fix error message bubbling up on seed error Jun 3, 2019

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

@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 tgriesser:master Jun 3, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 88.69%
Details
@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

Thanks a lot for your contribution!

@risseraka

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

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

(Impressive speed by the way. 😅)

@kibertoad

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

@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

This comment has been minimized.

Copy link
Collaborator

commented Jun 17, 2019

Released in 0.18.0-next1

@risseraka

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

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
You can’t perform that action at this time.