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

Update Error handling crate to thiserror #613

Closed
wusyong opened this issue May 23, 2020 · 12 comments · Fixed by #621 or #623
Closed

Update Error handling crate to thiserror #613

wusyong opened this issue May 23, 2020 · 12 comments · Fixed by #621 or #623

Comments

@wusyong
Copy link
Member

wusyong commented May 23, 2020

Is your feature request related to a problem? Please describe.
I would like to open a PR, but looks like it's better to file an issue first. As failure is already deprecated, it seems error-chain will be the same even it hasn't updated the badge to toml file yet. So I think it's better update to a more modern error handling type crate like thiserror as soon as we have time. Especially thiserror is already v1.0 and its interface should be lighter and intuitive.

Describe the solution you'd like
Replace error-chain with thiserror v1.0.

Describe alternatives you've considered
As a crate being binary/executable, maybe it doesn't need to have a well-typed error as long as there's way to retrieve actual error. In this case, we can even use anyhow which can make error handling even more easier.

Additional context
Add any other context or screenshots about the feature request here.

@nothingismagick
Copy link
Member

@tensor-programming was discussing moving to anyhow and thiserror - maybe wait for his feedback before starting a PR.

@wusyong
Copy link
Member Author

wusyong commented May 26, 2020

Looks like anyhow + thiserror is also a good approach too. We can just return all Result with anyhow::Result, while we can still have custom error type when needed without bothering every kind to error types.

@nothingismagick
Copy link
Member

One thing tho - I was running some frida-trace stuff, and I'd like to be able to totally disable ALL error reporting in builds. Thoughts?

@wusyong
Copy link
Member Author

wusyong commented May 26, 2020

Do you mean something like abort on panic? (which will not unwind the error stack)

@nothingismagick
Copy link
Member

well - its a tradeoff - i don't want to crash if it can be avoided, but i also don't want to give reversers / hackers information that will inform them about what is going on under the hood

@wusyong
Copy link
Member Author

wusyong commented May 26, 2020

Yeah, in that case we could add this to abort on panic in release build.

[profile.release]
panic = "abort"

Btw, this probably should be a topic on its own. Maybe split this into another issue later.

@nothingismagick
Copy link
Member

agreed - its something definitely for the docs - will create that over there

@nothingismagick
Copy link
Member

Do you want to start working on the PR @wusyong ?

@wusyong
Copy link
Member Author

wusyong commented May 26, 2020

Sure, I would like to take it.

@tensor-programming
Copy link
Member

tensor-programming commented May 28, 2020

I've started work on my own PR not seeing yours or this issue (sorry about that). If you want; we can merge the two; I don't want to minimize the work you've already done.

I've started with the bundler and in doing so also cleaned up some of the old error codes. Ill have you help me on the actual lib crates; if you don't mind. (we can adapt the work you've already done.)

@wusyong
Copy link
Member Author

wusyong commented May 28, 2020

Thanks, I think I can leave bundler untouched.

@tensor-programming
Copy link
Member

tensor-programming commented May 29, 2020

Thank you, sorry again. You did some great work on #621.

tensor-programming pushed a commit that referenced this issue May 29, 2020
…#613 (#621)

* Replace error-chain with thiserror in util crate

* Replace errorchain with anyhow/thiserror in api

* Replace with anyhow/thiserror in updater

* Replace with anyhow/thiserror in tauri

* Fix error handling on windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants