Export a top-level Error enum from construct_runtime!

[dandanlen]

Is there an existing issue?

• I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• This is not a support question.

Description of bug

Not technically a bug, more of feature request, but then there is no other category of issue, and I opened a stackexchange issue but got little traction beyond "why would you need this?".

---

Given that the construct_runtime macro exports top-level Call and Event enums that group the pallet-level equivalents, it seems reasonable to expect that there would also be a top-level Error enum to allow easy encoding and decoding of runtime errors.

The specific use-case I have in mind is an api that allows execution of a runtime::Call with response type of Result<runtime::Event, runtime::Error>, where runtime::Error contains a useful error message (or at least a structured enum that can be matched on).

Encoding the Call: easy. Decoding the Events: easy. Decoding the Error.. not so easy.

Right now the only option (as far as I can tell) to get the details of the Error involves parsing the metadata to pull out the error details, which is not the most intuitive or ergonomic. If there is a better alternative, please let me know!

I appreciate this might be non-trivial to add, but it would be great if it could be considered.

Steps to reproduce

No response

[bkchr]

The specific use-case I have in mind is an api that allows execution of a runtime::Call with response type of Result<runtime::Event, runtime::Error>, where runtime::Error contains a useful error message (or at least a structured enum that can be matched on).

As already on SE, I don't get where you want to use this api? Where would this kind of api live and do?

[dandanlen]

It's the same use case as polkaJs and subxt - but both are general-purpose client libraries. If I want to write one specifically for my runtime, what I have suggested above might be helpful.

[bkchr]

Subxt already has an issue for exactly what you want: paritytech/subxt#313

[dandanlen]

So... great! Clearly I'm not the only one who thinks this would be useful. The issue has been open in subxt for over a year, though, so apparently not getting much love. Moreover, like I said, subxt is a general purpose library, and for various reasons we'd rather write our own simple client. Subxt is a general purpose library that generates client code from the type metadata. It's very clever and magical. But it's intended for client libraries that don't necessarily have access to the source code of the target chain.

For anyone writing a client for their own chain, we shouldn't need to generate it from the type metadata: we have all the type info we need in the source code, and we have the rust compiler to ensure its correctness. The only thing we're missing is a slightly more useful runtime Error type - hence my suggestion.

I don't really understand - is there some specific reason you think it's a bad idea?

[bkchr]

So you want to use this error type in your node? Or where exactly? Or only in your runtime?

[dandanlen]

My use case is outside the runtime (but with access to the runtime crate: so I can use the concrete types of my runtime). I want to create a type-safe wrapper around the jsonrpc interface to submit extrinsics. And I want to be able to return a useful error when a call fails.

I can submit an extrinsic with an encoded runtime::Call, get the hash, and then watch for runtime::Events for that extrinsic. If I get the Event ExtrinsicFailed then I know there was an error - If the error occurred in the pallet code, I can pull out the ModuleError, which is defined as:

/// Reason why a pallet call failed.
#[derive(Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)]
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
pub struct ModuleError {
	/// Module index, matching the metadata module index.
	pub index: u8,
	/// Module specific error value.
	// NB. MAX_MODULE_ERROR_ENCODED_SIZE is 4 for some reason
	pub error: [u8; MAX_MODULE_ERROR_ENCODED_SIZE], 
	/// Optional error message.
	#[codec(skip)]
	#[cfg_attr(feature = "std", serde(skip_deserializing))]
	pub message: Option<&'static str>,
}

But note the message is not encoded. So in order to get any useful information about the error (ie. the originating pallet::Error), I need to write some additional code to parse the module index and error index. And it seems like a top-level runtime::Error enum would help: I could just do something like runtime::Error::decode([module_error.index, module_error.error]).

I feel like this would obviously be quite useful, in the same way that being able to refer to calls and events can be useful. I don't understand why errors do not follow the same approach as used for calls and events - was this a conscious decision? Is there a better alternative for what I'm trying to do?

[bkchr]

I don't understand why errors do not follow the same approach as used for calls and events - was this a conscious decision?

Yes it was. Otherwise we would have required to add type Error to each pallet Config trait with a bound type Error: From<crate::Error>. Instead of this we are being able to return pallet related errors in a much simpler way.

I also get what you are trying to do, but this goes a little bit against the design decisions of Substrate. Substrate is build in a way that the node and runtime are separate. They are only talking to each other via defined interfaces/apis. When you would start using the Error enum from the runtime it would may break after a runtime upgrade. Imagine a runtime upgrade which adds a new error variant that is triggered. Your node would not be able to "parse" this as it doesn't know this variant.

If you are watching for the runtime events any way, getting the error and index isn't hard. What you could do is to add support for your requested feature to frame-metadata so that it can give you the concrete error or the error docs for a given index & error. This also shouldn't be that hard. WDYT?

[dandanlen]

Hey, thanks for the reply!

I guess this makes sense but then similar arguments seem to apply to pallet calls, events and origins. And we could have the Error type for convenience but still use DispatchError where it makes sense to do so. I'm not suggesting to change the entire node/runtime interface!

TLDR I can see why it was done the way it was, but I still think a top-level RuntimeError enum would be useful in some cases, for similar reasons that top-level RuntimeEvent/Call/Origins are useful. I don't see why Errors are an exception.

The suggested compromise with frame-metadata is definitely a good alternative though, thanks. How can I request the feature / add my support?

[bkchr]

The suggested compromise with frame-metadata is definitely a good alternative though, thanks. How can I request the feature / add my support?

You can open an issue here: https://github.com/paritytech/frame-metadata

[dandanlen]

Thanks, I found this - paritytech/frame-metadata#43

Looks like it might fit our needs.