utility.dispatch_as error handling

[sander2]

The utility.dispatch_as function hides errors: https://github.com/paritytech/substrate/blob/8186c519a861ea519ed3dc4391f6e13f64300ce6/frame/utility/src/lib.rs#L392-L405

This is problematic when you have dispatch_as calls within batch_all calls - the batch will succeed despite errors. I assume that you won't want to change the behavior of existing extrinsics, so it would be really nice if a new function could be added that propagates the error. I'm struggling to come up with a suitable name to distinguish it from the extisting dispatch_as though..

[ggwpez]

Yea it looks a bit weird. That call is more of an force_dispatch_as, since it ignores the error.
I think it makes sense to change, since it can still be wrapped in a force_batch to ignore the error - if desired.

[xlc]

Not saying this is desired behaviour but it is intended behaviour consistent with other similar code.

The reason is that the disaptch_as have successfully dispatched a call and therefore the Ok response. If for example the origin check failed, it will then respond Err. It is not good to merge errors which could cause more confusion.

[sander2]

Not saying this is desired behaviour but it is intended behaviour consistent with other similar code.

The reason is that the disaptch_as have successfully dispatched a call and therefore the Ok response. If for example the origin check failed, it will then respond Err. It is not good to merge errors which could cause more confusion.

Yea you can certainly argue that it's working as intended. I'd be fine with leaving the existing function untouched. You can consider this a feature request rather than a bug report. Just to illustrate the need of a dispatch_as variant that propagates errors, let me explain how I ran into this issue.

I had a batch of ~50 calls initializing a new testnet. I wrapped the calls in batch_all since I wanted an all-or-nothing approach - some of the calls would only have the desired behavior if all of the previous calls succeeded. However, there was a failure hidden by some of the dispatch_as calls within the batch, and as a result the batch was committed, resulting in an unintended chain state that was very difficult to fix.

[0xmovses]

Any reason why the linked PR was not merged even though it was approved? paritytech/substrate#13387

Do we still want the changes in submitted in the PR?