JoshOrndorff / librho

A Standard Library for Rholang
Apache License 2.0
12 stars 6 forks source link

error handling conventions #14

Open dckc opened 5 years ago

dckc commented 5 years ago

Keeping track of error handling conventions is becoming a real challenge:

Boolean

            if (v + x > v) {
              valueStore!(v + x) | success!(true)
            } else {
              //overflow
              valueStore!(v) | success!(false)
            }

-- https://github.com/rchain/rchain/blob/dev/casper/src/main/rholang/NonNegativeNumber.rho

Maybe using list

                    if (success) {
                      return!([destPurse])
                    } else {
                      return!([])
                    }

-- https://github.com/rchain/rchain/blob/dev/casper/src/main/rholang/MakeMint.rho

Either, following scala and haskell

("Right", value)
("Left", error)

https://github.com/rchain/rchain/blob/dev/casper/src/main/rholang/Either.rho

downside of using tuples: no direct JSON mapping.

Waterken JSON convention

I used this one this week:

{ "=" : 42 }
{ "!" : "LPT1 on fire" }

https://github.com/dckc/RChain-API/blob/cli/rclient/src/main.js#L529 (oops... did I neglect to check in tools.rho???)

This idiom plays nicely with union types supported by flow and typescript:

type WebSendResult<T> = { "=": T } | { "!": string }

Meanwhile, flow (and I think typescript) also do disjoint unions this way:

type Success = { success: true, value: boolean };
type Failed  = { success: false, error: string };

https://flow.org/en/docs/types/unions/#toc-disjoint-unions

status channel, out parameter

This one is pretty wonky: if one channel has "Success" then you expect the value on another channel.

                          match splitResult {
                            []        => { status!("Overdraft") }
                            [payment] => { destination!(payment) | status!("Success") }

https://github.com/rchain/rchain/blob/dev/casper/src/main/rholang/BasicWallet.rho

Separate resolve, reject channels

I used this a couple weeks ago when I was doing voting and lockers.

pro: non-local returns, a la exceptions; e.g. decode in rlp.rho https://github.com/dckc/RChain-API/blob/master/examples/ethSig/rlp.rho#L221-L312

con: unless our garbage collector is pretty smart, leaves a for listening for whichever channel doesn't fire.

cf. @JoshOrndorff Oct 24 https://github.com/JoshOrndorff/librho/issues/7#issuecomment-432743231

Java exception

If you pass the wrong number of bytes to at least one of the crypto functions, you get a Java-level error and your rholang process (all of them? just one?) just stops.

This is really, really bad. The built-in methods really need to be total.

dckc commented 5 years ago

I guess I like this:

        // most Vault methods return an `Either[String, A] = (false, String) \/ (true, A)`

-- https://github.com/rchain/rchain/blob/dev/rholang/examples/vault_demo/0.create_genesis_vault.rho#L24

I think tuples should get mapped to JSON as {_1: false, _2: "abc" }.

JoshOrndorff commented 5 years ago

@EdEykholt

dckc commented 5 years ago

I just noticed this sequence of @Either calls in RevVault.rho:

                RevAddress!("validate", revAddress, *revAddressValid) |
                @Either!("fromNillableError <-", *revAddressValid, *revAddressValidEither) |

                @Either!("fromBoolean", amount >= 0, "Amount must be non-negative", *amountNonNegative) |

                @AuthKey!("check", *authKey, (*_revVault, ownRevAddress), *authKeyValidCh) |
                @Either!("fromBoolean <-", *authKeyValidCh, "Invalid AuthKey", *authKeyValidEitherCh) |

                @Either!("productR <-", *revAddressValidEither, *amountNonNegative, *parametersOkCh) |
                @Either!("productR <-", *parametersOkCh, *authKeyValidEitherCh, *parametersAndAuthOkCh) |

This function style is somewhat nice to read in that it's easy to see that it's total, but it's expensive. Each call after the initial failure costs a couple COM events and a match, yes?

Separate resolve / reject channels would be more efficient, yes?

ref git commit discussion