haskell / haskell/cabal

Better exception throwing machinery

Open
#10,747 2 comments 0 reactions 0 assignees View on GitHub
needs triage
Dominant language
Haskell
Stars
1.7k
Forks
750
Avg merge
4d 3h
Merged PRs (30d)
28

Description

# Context

When a `CabalException` or [`CabalInstallException`](https://github.com/haskell/cabal/blob/1082c0bb4e498fc9ea3b7b8cbfa78a99f2edb3b9/cabal-install/src/Distribution/Client/Errors.hs#L38) is thrown via [`dieWithException`](https://github.com/haskell/cabal/blob/1082c0bb4e498fc9ea3b7b8cbfa78a99f2edb3b9/Cabal/src/Distribution/Simple/Utils.hs#L425-L447), 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`](https://github.com/haskell/cabal/blob/1082c0bb4e498fc9ea3b7b8cbfa78a99f2edb3b9/cabal-install/src/Distribution/Client/ProjectConfig.hs#L1053-L1056), 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](https://rustc-dev-guide.rust-lang.org/diagnostics.html)

## 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](https://github.com/haskell/cabal/commit/d2dd158ba833f5c3f633d7c79572dbcfbede1447) 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:

```haskell
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`](https://hackage.haskell.org/package/base-4.21.0.0/docs/Control-Exception.html#t:SomeException)) which replaces `VerboseException`:

```haskell
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`](https://github.com/parsonsmatt/annotated-exception#readme)) after [my first attempt](https://github.com/9999years/cabal/commit/5f4750a15abd6d47bf5a9414df35af8f76c7f7fe) 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.

Contributor guide

Open the contributing guide

Research direction

Start by reading dieWithException in Cabal/src/Distribution/Simple/Utils.hs, the CabalInstallException definition in cabal-install/src/Distribution/Client/Errors.hs, and the BadPackageLocations case in cabal-install/src/Distribution/Client/ProjectConfig.hs. Compare the linked ProjectImport integration test and the proposed SomeCabalException design. Done means a reviewed exception mechanism that centralizes error codes, preserves structured catching, and provides verbosity and output markers.

Written by the indexing model from the issue text.

Assessment

Tech stack
haskell
Domain
build-system
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.