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

Return errors in keeper methods instead of panicking #15555

Closed
14 tasks
facundomedica opened this issue Mar 27, 2023 · 6 comments
Closed
14 tasks

Return errors in keeper methods instead of panicking #15555

facundomedica opened this issue Mar 27, 2023 · 6 comments
Labels
C:x/auth T:tech debt Tech debt that should be cleaned up

Comments

@facundomedica
Copy link
Member

facundomedica commented Mar 27, 2023

Summary

Now that we are using the new storeService that returns an error for all methods we can return them instead of panic. It might need some other changes like stop using Must... kind of functions

Problem Definition

We panic in many different places.

Proposal

Stop panicking and be responsible for your errors

Module list

@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Mar 27, 2023
@tac0turtle tac0turtle added C:x/auth T:tech debt Tech debt that should be cleaned up and removed needs-triage Issue that needs to be triaged labels Mar 27, 2023
@tac0turtle
Copy link
Member

for begin and endblock we should have a way for users to say which error should halt the chain and which shouldnt. lots of benign issues could be avoided

@facundomedica
Copy link
Member Author

Maybe like a type "CriticalError", so if you get this type panic, and if not just log it

@tac0turtle
Copy link
Member

yea that could work. this is the epic that eludes to something as well. #12985.

@alexanderbez
Copy link
Contributor

Yes, totally in favor of errors where possible. I imagine 90% of keeper/module code can error. The only thing that needs to be able to panic is Begin/EndBlock and Message handlers.

@julienrbrt julienrbrt changed the title (x/auth): return errors in keeper methods instead of panicking Return errors in keeper methods instead of panicking May 10, 2023
@chandiniv1
Copy link
Contributor

@facundomedica @tac0turtle can I take this issue?

@likhita-809
Copy link
Contributor

@tac0turtle closing this issue. As it is already addressed by #18636

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/auth T:tech debt Tech debt that should be cleaned up
Projects
None yet
Development

No branches or pull requests

5 participants