Skip to content

feat(store): improve errors#518

Merged
polydez merged 2 commits intonextfrom
polydez-improve-node-errors
Oct 21, 2024
Merged

feat(store): improve errors#518
polydez merged 2 commits intonextfrom
polydez-improve-node-errors

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
Collaborator

@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.

@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
@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