Adding errors to actions to help negative testing

[MaximilianAlgehed]

Adding errors to action types makes it easier to build clean models that deal with negative testing by allowing you to clearly distinguish the failing and correct cases. The thing I'm not yet happy about is having to sprinkle Rights in the results. Suggestions welcome!

Checklist:

• Check source-code formatting is consistent

[MaximilianAlgehed]

@abailly-iohk any input on this would be welcome.

[ghost]

This is a significant (eg. breaking) change the advantages of which are not clear to me: From the RegistryModel I don't see what the added complexity in additional type parameters and mandatory Either results improves in the DX. It seems the only significant change is in the postCondition and postConditionOnFailure implementations and the added value does not seem to be worth the effort to my eyes.
Is there a model somewhere, perhaps in Plutus land, where this way of "first-class" handling of errors would be more beneficial?

[MaximilianAlgehed]

This is a significant (eg. breaking) change the advantages of which are not clear to me: From the RegistryModel I don't see what the added complexity in additional type parameters and mandatory Either results improves in the DX.

The mandatory Either result is annoying - I agree. I would like to find a nice way to get around that. One way would be to have a monad transformer RunMonad e that is basically an error transformer that would hide this.

It seems the only significant change is in the postCondition and postConditionOnFailure implementations

Yes, this is the idea. The point is that in the case where there can be errors in the model you can add those errors without having to clutter the model with the possibility of error. The whole point is that actions can fail but that failure shouldn't appear in the model - so nextState shouldn't have to deal with the fact that an action can fail.

the added value does not seem to be worth the effort to my eyes.

Well, the effort has already been made 😅 and the migration guide is basically "add Right and a type parameter" so I don't see that as being very onerous.

Is there a model somewhere ... where this ... would be more beneficial?

There are examples in djed that we can't show yet. But let's do a hypothetical example. You have some action InitNewThing :: SomeParameters -> Action MyModel ThingHandle (by the old model) and your nextState uses the symbolic Var ThingHandle to associate something in SomeParameters to ThingHandle (e.g. a name in SomeParameters is associated with a contract instance in ThingHandle, or a hash in ThingHandle or whatever). Consequently, your MyModel contains a bunch of references to Var ThingHandle that you use in various places in perform (e.g. in the case for some DoThing :: Var ThingHandle -> Action MyModel () action). Imagine you want to add negative testing to this, now every Var ThingHandle will be a Var (Either SomeError ThingHandle) - which will clutter your entire model! Yuck! With this change, you don't have this any more, the action will be InitNewThing :: SomeParameters -> Action MyModel SomeError ThingHandle and all the Var ThingHandle will remain as such.

[ghost]

Well, if this ship has sailed already, then no need to wait for me :)

[ghost]

How about having an ActionError s associated type family instead of an additional type parameter? Seems to me the blast radius of the change would be smaller and in general more consistent with the overall approach we have so far followed. WDYT @MaximilianAlgehed ?

[MaximilianAlgehed]

@abailly-iohk I'm off for the next ~2 weeks so I'll let @UlfNorell handle this.

[MaximilianAlgehed]

But without having tried ut our thought about it I think it sounds like a good idea!

[ghost]

Great, I will give it a shot as I might have some bandwidth available for the rest of the week :)

[ghost]

I had a quick stab at replacing class' type variable with an associated type-family but did not go far yet. Will try again today.

[UlfNorell]

We had a go at the associated type and it seems to work quite well. Now you don't get the Either in perform unless you have defined an error type.

Some documentation still todo.

[MaximilianAlgehed]

@abailly-iohk what do you think about this now?

[ghost]

I like the way this is now done much more, thanks for taking into account my suggestion. Minor comments left which could be addressed later.