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

Retrieve errors when exitOnError is set to false #48

Closed
SamVerschueren opened this issue Jan 30, 2017 · 14 comments
Closed

Retrieve errors when exitOnError is set to false #48

SamVerschueren opened this issue Jan 30, 2017 · 14 comments

Comments

@SamVerschueren
Copy link
Owner

From @okonet

I've just tried to integrate it and can't wrap my head on how do I display all errors when all tasks complete. When using default renderer, it just continues silently now, so I was expecting the error argumemt in the catch to contain all my errors. Looking at the code it is not the case. Is there a way of accessing it somehow after the execution? Should it be some internal state I have to take care of? Please point me in the right direction.

Because exitOnError is set to false, at the end, the .catch function is not being invoked and you end up in then. Because the result of the promise is the context object, we could add an errors array and push all the errors into that list. A downside of this approach though is that we should make it a read-only property so that people can't overwrite that property.

@okonet @frederickfogerty thoughts?

@okonet
Copy link
Contributor

okonet commented Jan 30, 2017

The problem with then is that I don't have access to ctx in it anymore. At least it looks like no arguments is being passed there and this is also referring to undefined.

I think having all errors in one place and still be able to catch them would be a great help for such use cases. But I'm okay with implementing it myself as well. But it seems the current API doesn't really allow this.

@SamVerschueren
Copy link
Owner Author

Yes, then should return the context. See also this test.

@SamVerschueren
Copy link
Owner Author

@okonet So from what I understand is that you want the main process to throw after all the tasks have executed so you would end up in the catch?

@okonet
Copy link
Contributor

okonet commented Jan 31, 2017

Yeah, I think that would be the appropriate behavior for it.

@SamVerschueren
Copy link
Owner Author

Yeah, that actually makes sense. Just wondering what I should throw though. A custom error object and attach the error list.

const err = new ListrError('Some tasks might have failed');
err.errors = errorList;

throw err;

@okonet
Copy link
Contributor

okonet commented Jan 31, 2017

That makes sense to me. The err.errors can be then accessed in catch(err) block, right?

@SamVerschueren
Copy link
Owner Author

Yes exactly.

@SamVerschueren
Copy link
Owner Author

And maybe it makes sense to also attach the context to the error so you can access that one via err.context. Otherwise it would not be possible to do that.

@okonet
Copy link
Contributor

okonet commented Jan 31, 2017

Yes, it totally makes sense!

@frederickfogerty
Copy link

Looks good to me 👍

@SamVerschueren
Copy link
Owner Author

Released in 0.11.0. Sorry for the late response, was quite busy.

@okonet
Copy link
Contributor

okonet commented Feb 19, 2017

Awesome! Thanks for the work. I'm integrating it now.

@SamVerschueren
Copy link
Owner Author

Let me know how it went !

@okonet
Copy link
Contributor

okonet commented Feb 19, 2017

Worked like a breeze: lint-staged/lint-staged#131. Thanks again for your work!

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

No branches or pull requests

3 participants