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

feat(store): improve errors #518

Merged
merged 2 commits into from
Oct 21, 2024
Merged

feat(store): improve errors #518

merged 2 commits into from
Oct 21, 2024

Conversation

polydez
Copy link
Contributor

@polydez polydez commented Oct 18, 2024

In this PR we improve error statuses returned by the some store API.

Initially it was needed by #504 (I wanted to be able to process "account not in DB" errors), but finally it became unused due to changed solution. But it might make our API better, so I propose to keep these changes.

@polydez polydez marked this pull request as ready for review October 18, 2024 15:23
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good improvement.

There are still possibilities where the mapping might be the wrong default for some situations; e.g. if we have bad internal state and attempt to retrieve an unknown block then that shouldn't be exposed.

Though I think this will always remain the case; and this PR at least restrains the default non-internal mappings to a smaller set.

crates/store/src/server/api.rs Show resolved Hide resolved
@polydez
Copy link
Contributor Author

polydez commented Oct 21, 2024

There are still possibilities where the mapping might be the wrong default for some situations; e.g. if we have bad internal state and attempt to retrieve an unknown block then that shouldn't be exposed.

Yes, for such cases we can map it to whatever we need. :)

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you!

@polydez polydez merged commit d920148 into next Oct 21, 2024
8 checks passed
@polydez polydez deleted the polydez-improve-node-errors branch October 21, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants