fix: use custom errors for the runtime #197
Conversation
dignifiedquire
force-pushed
the
fix-custom-error-runtime
branch
from
036e230
to
bbcac79
Compare
| pub enum Error { | ||
| // SysErrIllegalActor | ||
| #[error("Method must validate caller identity exactly once")] | ||
| CallerAlreadyValidated, | ||
| // SysErrForbidden | ||
| #[error("caller {0} ist not one of supported")] | ||
| UnsupportedCallerAddress(Address), | ||
| // SysErrForbidden | ||
| #[error("caller cid type {0} not one of supported")] | ||
| UnsupportedCallerType(Cid), | ||
| #[error("syscall: {0}")] | ||
| Syscall(#[from] ErrorNumber), | ||
| // ErrIllegalState | ||
| #[error("failed to create state; expected empty array CID, got: {0}")] | ||
| RootNotEmpty(Cid), | ||
| #[error("encoding: {0}")] | ||
| CborStore(anyhow::Error), | ||
| #[error("no state: {0}")] | ||
| NoState(#[from] NoStateError), | ||
| #[error("exit code: {0}")] | ||
| NonZeroExitCode(ExitCode), | ||
| // SysErrIllegalActor | ||
| #[error("runtime.send() is not allowed")] | ||
| SendNotAllowed, | ||
| #[error("actor delete: {0}")] | ||
| ActorDelete(#[from] fvm::error::ActorDeleteError), | ||
| } |
There was a problem hiding this comment.
We should define errors for specific syscalls.
There was a problem hiding this comment.
Otherwise the user has to deal with a bunch of non-specific errors.
There was a problem hiding this comment.
Could you expand on this a little, do you mean having one error variant per syscall error, or just a general Syscall(SyscallError) variant, or sth else?
There was a problem hiding this comment.
Ah, sorry. I mean different error types for different runtime functions.
|
@Stebalien @anorth any other thoughts on how/where to store the mapping of errors to exit codes? Does the way I did it make sense? should it be reusable for the mockvm somehow? |
|
Where there's a clean conversion, I'd implement |
|
Updated: this will be updated once #214 has landed |
Fixes #196
This is not ready to be merged, as the exit code mappings are currently screwed up, but opening it for discussion on how this mapping should be implemented if we set things up like this.
cc @anorth @Stebalien