(minimal) Add EnvBase::Error, remove CheckedEnv, make Env methods return Error #638
Conversation
This file contains 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
graydon
changed the title
dmkozh
approved these changes
Jan 23, 2023
graydon
added a commit
to stellar/rs-soroban-sdk
that referenced
this pull request
Jan 23, 2023
This is a "minimal" version of #833, accompanying / adapting to stellar/rs-soroban-env#638. If this minimal approach is preferred, this PR and stellar/rs-soroban-env#638 should be merged together. Otherwise the more-invasive pair #833 and stellar/rs-soroban-env#633 should be merged together. This one does much less rearranging and makes no difference to the emitted code at all. It just pushes around where the non-infallible `Env::Error` cases get escalated: formerly it happened in `impl Env for CheckedEnv`, now there's a new internal SDK function `internal::reject_err` that does the work. Infallible errors also need to be explicitly unwrapped in a bunch of places but this is just a type-level thing, operationally it's a no-op. (I should also note: I could do an "even more minimal" version of this where `soroban_sdk::env::Env` _does not implement_ `soroban_env_common:env::Env`, in which case the "error rejections" could be buried in the macro-generated code in the SDK and we could remove all the `.unwrap_infallible` calls in the SDK bodies. I don't know how important it is for the SDK's Env to implement the common Env.)
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a "minimal" version of #633 that does not touch the
TryFromValtrait, just movesCheckedEnv's error functionality intoEnvby definingEnv::Errorand making every method returnResult<T, Env::Error>.The main differences are:
TryFromValstill has to define its ownErrortype.HostErrordowngraded to aConversionErrorin such aTryFromVal.Env::escalate_status_to_errororMapErrToEnv, because it does those downgrades / information losses.The last point is evident in the corresponding SDK change: stellar/rs-soroban-sdk#837