|
| 1 | +# Status |
| 2 | +- [x] Adopted 2025/02/10 |
| 3 | + |
| 4 | + |
| 5 | +# Required Reading |
| 6 | + |
| 7 | +- [ReaderT design pattern](https://tech.fpcomplete.com/blog/2017/06/readert-design-pattern/). |
| 8 | +- [RIO](https://tech.fpcomplete.com/haskell/library/rio/) |
| 9 | + |
| 10 | +# Context |
| 11 | +In `cardano-cli` we are using `ExceptT someError IO someResult` pattern 292 times in function types in our codebase for different error types. |
| 12 | + |
| 13 | + |
| 14 | + |
| 15 | +The vast majority of these errors are not used to recover, they are propagated and reported to the user. |
| 16 | +```haskell |
| 17 | +main :: IO () |
| 18 | +main = toplevelExceptionHandler $ do |
| 19 | + envCli <- getEnvCli |
| 20 | + co <- Opt.customExecParser pref (opts envCli) |
| 21 | + orDie (docToText . renderClientCommandError) $ runClientCommand co |
| 22 | + |
| 23 | +... |
| 24 | + |
| 25 | +runClientCommand :: ClientCommand -> ExceptT ClientCommandErrors IO () |
| 26 | +runClientCommand = \case |
| 27 | + AnyEraCommand cmds -> |
| 28 | + firstExceptT (CmdError (renderAnyEraCommand cmds)) $ runAnyEraCommand cmds |
| 29 | + AddressCommand cmds -> |
| 30 | + firstExceptT AddressCmdError $ runAddressCmds cmds |
| 31 | + NodeCommands cmds -> |
| 32 | + runNodeCmds cmds |
| 33 | + & firstExceptT NodeCmdError |
| 34 | + ByronCommand cmds -> |
| 35 | + firstExceptT ByronClientError $ runByronClientCommand cmds |
| 36 | + CompatibleCommands cmd -> |
| 37 | + firstExceptT (BackwardCompatibleError (renderAnyCompatibleCommand cmd)) $ |
| 38 | + runAnyCompatibleCommand cmd |
| 39 | +... |
| 40 | +``` |
| 41 | +- As a result we have a lot of errors wrapped in errors which makes the code unwieldly and difficult to compose with other code blocks. See image below of incidences of poor composability where we use `firstExceptT` (and sometimes `first`) to wrap errors in other errors. |
| 42 | + |
| 43 | + |
| 44 | + |
| 45 | +- The ExceptT IO is a known anti-pattern for these reasons and others as per: |
| 46 | + - https://github.com/haskell-effectful/effectful/blob/master/transformers.md |
| 47 | + - https://tech.fpcomplete.com/blog/2016/11/exceptions-best-practices-haskell/ |
| 48 | + |
| 49 | + |
| 50 | +# Proposed Solution |
| 51 | + |
| 52 | +I propose to replace `ExceptT someError IO a` with [RIO env a](https://hackage.haskell.org/package/rio-0.1.22.0/docs/RIO.html#t:RIO). |
| 53 | + |
| 54 | + |
| 55 | +### Example |
| 56 | + |
| 57 | +Below is how `cardano-cli` is currently structured. `ExceptT` with errors wrapping errors. However the errors ultimately end up being rendered at the top level to the user. |
| 58 | + |
| 59 | +```haskell |
| 60 | + |
| 61 | +-- TOP LEVEL -- |
| 62 | + |
| 63 | +data ExampleClientCommand = ClientCommandTransactions ClientCommandTransactions |
| 64 | + |
| 65 | +data ExampleClientCommandErrors |
| 66 | + = CmdError CmdError |
| 67 | +-- | ByronClientError ByronClientCmdError |
| 68 | +-- | AddressCmdError AddressCmdError |
| 69 | +-- ... |
| 70 | +data CmdError |
| 71 | + = ExampleTransactionCmdError ExampleTransactionCmdError |
| 72 | +-- | AddressCommand AnyEraCommand |
| 73 | +-- | ByronCommand AddressCmds |
| 74 | +-- ... |
| 75 | + |
| 76 | +topLevelRunCommand :: ExampleClientCommand -> ExceptT ExampleClientCommandErrors IO () |
| 77 | +topLevelRunCommand (ClientCommandTransactions txsCmd) = |
| 78 | + firstExceptT (CmdError . ExampleTransactionCmdError) $ runClientCommandTransactions txsCmd |
| 79 | + |
| 80 | +-- SUB LEVEL -- |
| 81 | + |
| 82 | +data ClientCommandTransactions = DummyClientCommandToRun |
| 83 | + |
| 84 | +data ExampleTransactionCmdError |
| 85 | + = TransactionWriteFileError !(FileError ()) |
| 86 | + |
| 87 | +runClientCommandTransactions |
| 88 | + :: () |
| 89 | + => ClientCommandTransactions |
| 90 | + -> ExceptT ExampleTransactionCmdError IO () |
| 91 | +runClientCommandTransactions DummyClientCommandToRun = |
| 92 | + ... |
| 93 | + left $ |
| 94 | + TransactionWriteFileError $ |
| 95 | + FileError "dummy.file" () |
| 96 | +``` |
| 97 | + |
| 98 | +Proposed change: |
| 99 | + |
| 100 | +```haskell |
| 101 | +data ClientCommandTransactions = DummyClientCommandToRun |
| 102 | + |
| 103 | +data ExampleClientCommand = ClientCommandTransactions ClientCommandTransactions |
| 104 | + |
| 105 | +topLevelRunCommand :: ExampleClientCommand -> RIO () () |
| 106 | +topLevelRunCommand (ClientCommandTransactions txsCmd) = |
| 107 | + runClientCommandTransactions txsCmd |
| 108 | + |
| 109 | +runClientCommandTransactions |
| 110 | + :: HasCallStack |
| 111 | + => ClientCommandTransactions |
| 112 | + -> RIO () () |
| 113 | +runClientCommandTransactions DummyClientCommandToRun = |
| 114 | + ... |
| 115 | + throwIO $ |
| 116 | + CustomCliException $ |
| 117 | + FileError "dummy.file" () |
| 118 | +``` |
| 119 | + |
| 120 | +We have eliminated `data ExampleClientCommandErrors` and `data CmdError` and improved the composability of our code. |
| 121 | + |
| 122 | +### Pros |
| 123 | +- Additional [logging functionality](https://hackage.haskell.org/package/rio-0.1.22.0/docs/RIO.html#g:5) |
| 124 | +- Explicit environment dependencies e.g `logError :: HasLogFunc env => Text -> RIO env ()` |
| 125 | +- Better composability i.e no more errors that wrap errors (see above). |
| 126 | + |
| 127 | +### Cons |
| 128 | +- `RIO` is hardcoded to IO so we cannot add additional transformer layers e.g `RIO Env (StateT Int IO) a` |
| 129 | +- Implicit error flow. Errors are thrown via GHC exceptions in `IO`. See exception handling below. |
| 130 | + |
| 131 | +## Exception handling |
| 132 | + |
| 133 | +### Exception type |
| 134 | +```haskell |
| 135 | +data CustomCliException where |
| 136 | + CustomCliException |
| 137 | + :: (Show error, Typeable error, Error error, HasCallStack) |
| 138 | + => error -> CustomCliException |
| 139 | + |
| 140 | +deriving instance Show CustomCliException |
| 141 | + |
| 142 | +instance Exception CustomCliException where |
| 143 | + displayException (CustomCliException e) = |
| 144 | + unlines |
| 145 | + [ show (prettyError e) |
| 146 | + , prettyCallStack callStack |
| 147 | + ] |
| 148 | + |
| 149 | +throwCliError :: MonadIO m => CustomCliException -> m a |
| 150 | +throwCliError = throwIO |
| 151 | +``` |
| 152 | + |
| 153 | +The purpose of `CustomCliException` is to represent explicitly thrown, structured errors that are meaningful to our application. |
| 154 | + |
| 155 | +### Exception Handling Mechanism: Pros & Cons |
| 156 | + |
| 157 | +#### Pros |
| 158 | + |
| 159 | +1. **Unified Exception Type** |
| 160 | + |
| 161 | +- Simplifies Top-Level Handling: All errors are caught as `CustomCliException`. |
| 162 | +- Consistent Reporting: Ensures all errors are formatted uniformly via `prettyError`. |
| 163 | + |
| 164 | +2. **CallStack Inclusion** |
| 165 | + |
| 166 | +- Embeds CallStack to trace error origins, aiding debugging. |
| 167 | + |
| 168 | +3. **Polymorphic Error Support** |
| 169 | + |
| 170 | +- Flexibility: Wraps any error type so long as the required instances exist. |
| 171 | + |
| 172 | +#### Cons |
| 173 | +1. **Type Erasure** |
| 174 | + |
| 175 | +- Loss of Specificity: Existential quantification erases concrete error types, preventing pattern-matching on specific errors. That may make specific error recovery logic harder to implement. |
| 176 | + |
| 177 | +# Decision |
| 178 | +- Error handling: All errors will be converted to exceptions that will be caught by a single exception handler at the top level. |
| 179 | +- Top level monad: [RIO](https://hackage.haskell.org/package/rio-0.1.22.0/docs/RIO.html#t:RIO). |
| 180 | +- We agree not to catch `CustomCliException` except in the top-level handler. If the need for catching an exception arises, we locally use an `Either` or `ExceptT` pattern instead. |
| 181 | +- `CustomCliException` should only be thrown within the `RIO` monad. Pure code is still not allowed to throw exceptions. |
| 182 | + |
| 183 | +# Consequences |
| 184 | +- This should dramatically improve our code's composability and remove many unnecessary error types. |
| 185 | +- Readability concerning what errors can be thrown will be negatively impacted. However, `ExceptT` already lies about what exceptions can be thrown because it is not limited to the error type stated in `ExceptT`'s type signature. In other words, `IO` can implicitly throw other `Exception`s. |
| 186 | +- Initially, this will be adopted under the "compatible" group of commands so `cardano-cli` will have a design split temporarily. Once we are happy with the result we will propagate to the rest of `cardano-cli` |
0 commit comments