📑 dictionary lookups can return none#69
Merged
Merged
Conversation
added 6 commits
August 30, 2021 10:40
this error variant is now covered by `HandleError`.
aturon
reviewed
Aug 30, 2021
| pub enum DictionaryError { | ||
| /// A dictionary item with the given key was not found. | ||
| #[error("Unknown dictionary item")] | ||
| UnknownDictionaryItem, |
Member
There was a problem hiding this comment.
Quick question: what's the rationale for dropping the string paylaod here? Viceroy prints out the errors prior to converting them to ABI codes, and it seems helpful to provide that info.
Author
There was a problem hiding this comment.
ah! excellent point, thank you. i have fixed this in a0e80c6. ✨
aturon
reviewed
Aug 30, 2021
Member
aturon
left a comment
aturon
left a comment
There was a problem hiding this comment.
Looks good except for one small question!
> Viceroy prints out the errors prior to converting them to ABI codes, > and it seems helpful to provide that info. a wise point! this commit reintroduces the payload of `UnknownDictionaryItem`. Co-Authored-By: Aaron Turon <aturon@fastly.com>
Author
i'm guessing latent CI failure, we shall see ♻️ |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #68.
this branch makes some follow-up changes to #61, modifying the hostcall so that we return a
Nonestatus code when a dictionary entry does not exist.