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

Exhaustively define error types to improve correctness (security and stability) and maintainability #3851

Closed
frol opened this issue Jan 25, 2021 · 5 comments
Labels
A-chain Area: Chain, client & related C-enhancement Category: An issue proposing an enhancement or a PR with one. C-epic Category: an epic C-housekeeping Category: Refactoring, cleanups, code quality T-public-interfaces Team: issues relevant to the public interfaces team

Comments

@frol
Copy link
Collaborator

frol commented Jan 25, 2021

Ideally, every function should return only what it can potentially return, meaning that there should not be a huge enum with all possible errors in the world (like ErrorKind in chain), instead, there should be granular types (it can even be just a struct, not an enum, e.g. in Rust standard library there is AddrParseError, which is struct AddrParseError(()))

@frol: I would like to ensure the exhaustiveness of the potential error responses. How realistic is this approach in your opinion in regards to the core?

@bowenwang1996: It may be possible, but will likely take several months to complete

#3788 (comment)

Pros:

Cons:

@frol frol added C-enhancement Category: An issue proposing an enhancement or a PR with one. C-housekeeping Category: Refactoring, cleanups, code quality A-chain Area: Chain, client & related T-public-interfaces Team: issues relevant to the public interfaces team C-epic Category: an epic labels Jan 25, 2021
@volovyks
Copy link

@frol Is there a possibility that two or more functions will return the same error type? How we will manage them in this case?

@frol
Copy link
Collaborator Author

frol commented Jan 28, 2021

@volovyk-s If the methods share semantics, we can use the same error type, otherwise, we can compose the error types (extract common parts into a separate entity and implement simple wrappers for the specific needs of the functions)

@stale
Copy link

stale bot commented Jul 1, 2021

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Jul 1, 2021
@bowenwang1996
Copy link
Collaborator

@frol what are the action items here?

@stale stale bot removed the S-stale label Jul 1, 2021
@frol
Copy link
Collaborator Author

frol commented Aug 9, 2021

We strictly defined our JSON RPC layer errors and while we struggled to identify all the possible underlying error cases, we managed to handle those. This issue does not have any action items at the moment, so I am closing it.

@frol frol closed this as completed Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chain Area: Chain, client & related C-enhancement Category: An issue proposing an enhancement or a PR with one. C-epic Category: an epic C-housekeeping Category: Refactoring, cleanups, code quality T-public-interfaces Team: issues relevant to the public interfaces team
Projects
None yet
Development

No branches or pull requests

3 participants