feat: allow ActorErrors to carry optional data #1093
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1093 +/- ##
==========================================
- Coverage 89.32% 89.21% -0.12%
==========================================
Files 93 93
Lines 19590 19611 +21
==========================================
- Hits 17498 17495 -3
- Misses 2092 2116 +24
|
runtime/src/actor_error.rs
Outdated
| /// Returns the optional data that might be associated with the error | ||
| pub fn data(&self) -> &[u8] { | ||
| self.data.as_ref().map_or(&[] as &[u8], |d| d.data.as_slice()) | ||
| } |
There was a problem hiding this comment.
I realise this might have been written with some code in mind that we can't see here, but this seems like an oddly specific conversion to inline right here. Discarding the codec on purpose? Why not provide &Option<IpldBlock> and let the caller do what they need with it? Or some other variant.
There was a problem hiding this comment.
As far as I can tell, it's only used in a couple of tests.
(IIRC, it was written this way before we started using IpldBlock, and it should definitely be removed).
There was a problem hiding this comment.
Yup, I've removed it.
runtime/src/actor_error.rs
Outdated
|
|
||
| /// Creates a new ActorError. This method checks if the exit code is within the allowed range, | ||
| /// and automatically converts it into a user code. | ||
| pub fn checked(code: ExitCode, msg: String) -> Self { |
There was a problem hiding this comment.
I recognize this code as being copied (twice) from runtime/fvm.rs. I don't think it belongs here. It's appropriate only in the case of receiving an exit code from a send, not in various other places that exit codes propagate.
If you factored this match block out from the runtime as a helper function, adapt_callee_code or similar, that might be ok. But who's going to call it except the runtime? Links to how this is used might help.
There was a problem hiding this comment.
@anorth Good call, thank you. I'm going to refactor runtime/fvm.rs to use this method, instead, if that works for you. I'm also dropping the repeated copy -- just one checked method is sufficient, I think.
arajasek
force-pushed
the
asr/errors-with-data
branch
from
fb91885 to
9c445b7
Compare
arajasek
force-pushed
the
asr/errors-with-data
branch
from
9c445b7 to
610f0c5
Compare
|
@anorth I'd like your opinion on whether you think it'd be good to unify |
Extracted from the next branch.
Currently unused, but can be used to carry (optional) return data using the FVM3 SDK's exit method.