Better exception throwing machinery

[9999years]

Context

When a CabalException or CabalInstallException is thrown via dieWithException, we get a few benefits:

1. An error message that can (theoretically) be looked up in a manual is printed.
2. If some special options are run, output markers that enable only "stable" output to show up for cabal-testsuite.

$ cabal "-vnormal+markoutput" build
-----BEGIN CABAL OUTPUT-----
Error: [Cabal-7134]
-----END CABAL OUTPUT-----
-----BEGIN CABAL OUTPUT-----
No targets given and there is no package in the current directory. Use the target 'all' for all packages in the project or specify packages or components by name or location. See 'cabal build --help' for more details on target options.
-----END CABAL OUTPUT-----

As a result CabalInstallException is very important — it’s used by lots of modules — and as a result it’s very hard to include structured data in it, because importing the types you need to declare a variant with structured data nearly always results in import cycles.

So if you’re using a rich exception type like BadPackageLocations, you don’t get error codes and you don’t get output markers, so the integration tests are very painful, like this:

https://github.com/haskell/cabal/blob/1082c0bb4e498fc9ea3b7b8cbfa78a99f2edb3b9/cabal-testsuite/PackageTests/ProjectImport/DedupUsingConfigFromComplex/cabal.test.hs

Considerations for exception machinery

Centralization of error codes

We'd like all of the error codes to be defined and described in one place. (Or as few places as possible — currently they're split between CabalException and CabalInstallException.)

Rust solves this by writing a (detailed) description for each error code in one directory:

https://github.com/rust-lang/rust/blob/6c8347b9588a302afebb81a1ae6daa64ec37abd0/compiler/rustc_error_codes/src/error_codes/E0023.md

See also: "Errors and Lints" in the Rust Compiler Development Guide

Structured errors

We'd like to be able to throw and catch errors that contain richly-structured and typed data. CabalException and CabalInstallException largely just contain opaque String message fragments.

Richly-structured error types are critical — when we don't have these, the Cabal UX suffers (errors don't include error codes or other niceties) and the experience of Cabal maintainers suffers (integration tests are very difficult to write and difficult to maintain).

See the BadPackageLocations example above to see how this plays out in practice. Here's a commit showing how the experience improves if structured errors are able to hook into the VerboseException machinery. Additionally, it becomes possible for cabal-testsuite to automatically update the cabal.out file to reflect changes in the implementation, rather than relying on a programmer reading a (very noisy and long) readout of the test's output, locating the differences in the regular expression, and updating them to match. (The API used in that commit is not viable because it breaks the ability to catch these exceptions, but I believe the proposal outlined below does not suffer from this deficiency.)

Proposal

First, we add a class IsCabalException representing an exception like CabalException or CabalInstallException which can be pretty-printed and has an error code:

class
  ( Show e
  , Typeable e
  , Exception e
  , Pretty e
  ) => IsCabalException e where
  -- | Get this error's unique error code.
  getErrorCode :: e -> Int

Then, we add a type-erased SomeCabalException type (comparable to SomeException) which replaces VerboseException:

data SomeCabalException where
  SomeCabalException :: (IsCabalException e) => CallStack -> Verbosity -> e -> SomeCabalException

instance Exception SomeCabalException where
  -- ...

instance Exception CabalException where
  toException e = toException (SomeCabalException ... e)
  fromException someExn@(SomeException inner) = cast inner <|> do
    SomeCabalException inner' <- fromException someExn
    cast inner'

@parsonsmatt sketched out with the design for this system (inspired by annotated-exception) after my first attempt lost the ability to catch structured exceptions.

Using annotated-exception at work, I can confirm it works pretty nicely, but a notable pitfall is it becomes very easy to lose the annotations (in this case, the verbosity and callstack information) — because you can catch exceptions as the inner type, it's easy to write a catch clause that erases some of the exception's data.

[parsonsmatt]

After thinking about this a bit more:

You may not want Exception as a superclass, as that would allow you to use an un-adorned throwIO :: (Exception e) => .... If you want to require that IsCabalException e is thrown with extra information, then preserving something like dieWithException as the "official" throwing mechanism for "client facing errors" makes sense.

You can even put in a guardrail with an instance {Unsatisfiable,TypeError} (UseDieWithErrorMsg SomeType) => Exception SomeType that will report the proper behavior to use.

A slightly more gradual approach is to open up the instance Exception (VerboseException CabalException) behavior to instance (IsCabalException e) => Exception (VerboseException e). The methods used on CabalException are exceptionMessage and exceptionCode, which could be put into class members. You can still have TypeError msg => Exception typ instances here on to prevent people from throwing these without adornments.

I think the SomeCabalException pattern will result in the verbosity/callstack/timestamp being replaced in the case of a rethrow.

data E = E

instance IsCabalException E

woops = do
    dieWithException verbosity E
        `catch` \E -> 
            dieWithException verbosity otherException

This may be desirable if we're transforming the thrown exception type. But if we require folks to catch/throw with VerboseException, then we have an option of either preserving or replacing:

woops = do
    dieWithException verbosity E
        `catch` \original@(VerboseException _callStack _throwTime _verbosity E) ->
            if blah
                then dieWithException otherException
                else throwIO (VerboseException _callStack _throwTime _verbosity otherException)