Add instance Exception (Either e1 e2)

[Bodigrim]

cabal-install defines an orphan instance:

instance (Exception a, Exception b) => Exception (Either a b) where
  toException (Left e) = toException e
  toException (Right e) = toException e

  fromException e =
    case fromException e of
      Just e' -> Just (Left e')
      Nothing -> case fromException e of
        Just e' -> Just (Right e')
        Nothing -> Nothing

This can be expressed more eloquently as

instance (Exception a, Exception b) => Exception (Either a b) where
  toException = either toException toException
  fromException e = Left <$> fromException e <|> Right <$> fromException e

Is there a reason not to put it into base? Recently @hasufell and I needed such instance in another project, but had to workaround to avoid orphans.

According to Hackage Search the only packages affected are cabal-install-the-executable and hackport (because it includes cabal-install as a submodule). Both are executables without public library components, so there is no transitive breakage. Both are affected because of defining orphan instances and could be trivially patched by wrapping them in CPP. If the change is approved, I'll prepare patches once we know which version of base to use.

MR is available here: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11899

[parsonsmatt]

I'm in favor! I used something very much like that in my prio experiment for ergonomic checked exceptions.

It ends up being a recursive instance, so you can catch three types of exceptions at once: Either (Either A B) C. IMO it's a nicer interface than what catches provides. Compare:

action `catches`
     [ Handler (\(a :: A) -> handleA a
     , Handler (\(b :: B) -> handleB b
     , Handler (\(c :: C) -> handleC c
     ]

action `catch` \case
  Left (a :: A) -> handleA a
  Right (Left (b :: B)) -> handleB b
  Right (Right (c :: C)) -> handleC c

[hasufell]

There was a usecase of this in the tar package:

unpack :: Exception e => FilePath -> Entries e -> IO ()

checkSecurity :: Entries e -> Entries (Either e FileNameError)

checkPortability :: Entries e -> Entries (Either e PortabilityError)

With the proposed instance, you can do:

safeUnpack :: Exception e => FilePath -> Entries e -> IO ()
safeUnpack dir entries = unpack dir (checkPortability . checkSecurity $ entries)

where e at this point will be Either (Either e FileNameError) PortabilityError)

[hasufell]

It does have some resemblance of an open sum type for errors, doesn't it, like ExceptT, plucky (written by @parsonsmatt) and oops (which I think was never released).

It does seem like the proposed instance could potentially be inefficient for unknown error recursion depths. I briefly considered using ExceptT for tar, since I also use that for ghcup, but it did seem somewhat overkill for such a local issue, where you expect maybe a depth of 3 or 4 max.

[Bodigrim]

I've updated the proposal and prepared an MR: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11899

[ChickenProp]

I guess a slightly unusual thing here is that if you have an Either a a, fromException . toException will turn a Right into a Left. I'm not sure if that's a problem; I'd normally expect those to roundtrip, but is that something anyone's going to rely on?

Maybe possible to fix with something like

newtype LeftException a = LeftException a deriving newtype (Show, Exception)
newtype RightException a = RightExecption b deriving newtype (Show, Exception)

instance (Exception a, Exception b) => Exception (Either a b) where
  toException (Left e) = toException $ LeftException e
  toException (Right e) = toException $ RightException e

  fromException e =
    case fromException e of
      Just (LeftException e') -> Just (Left e')
      Nothing -> case fromException e of
        Just (RightException e') -> Just (Right e')
        Nothing -> Nothing

where LeftException and RightException aren't exported. But I don't know if that would cause other problems.

[Bodigrim]

See #135 (which was approved, but alas not implemented).

[ChickenProp]

Ah, in that case I guess we would want something that roundtrips, to avoid violating the undocumented law. I now realize that the code I gave above is overly complicated, and it would suffice just to use the default implementation.

But I don't know if that would cause other problems.

One potentially-problem that occurs to me is that cabal-install would now have a different instance.

[brandonchinn178]

If code threw Either Error1 Error2, and you do try @Error1, it wont catch it, right? I would be against this proposal, as I think beginners would see this as "this function throws two errors, and I can catch either of them".

[treeowl]

I would suggest either:

instance (Exception a, Exception b) => Exception (Either a b) where
  displayException (Left a) = "Left (" ++ displayException a ++ ")"

or just

instance (Typeable a, Typeable b, Show a, Show b) => Exception (Either a b)

[treeowl]

If code threw Either Error1 Error2, and you do try @Error1, it wont catch it, right? I would be against this proposal, as I think beginners would see this as "this function throws two errors, and I can catch either of them".

For that purpose, I'd rather see an explicit exception union type. It could be a biased union

data BU a b = BL a | BL b

-- Catching BU a b will catch both `a` and `b`, preferring `a`. Throwing will unwrap.

Or it could be an explicitly disjoint union, probably most simply like this:

data DU a b where
  DLeft :: PlainEq a b ~ False => a -> DU a b
  DRight :: PlainEq a b ~ False => b -> DU a b

type family PlainEq a b where
  PlainEq a a = True
  PlainEq _ _ = False 

Here no bias is required because the types are definitely distinct. However, a little bit of bias can be convenient even in the disjoint case:

data DU a b where
  DLeft :: a -> DU a b
  DRight :: PlainEq a b ~ False => b -> DU a b

There's no need to prove the types distinct unless both are thrown.

[ChickenProp]

If code threw Either Error1 Error2, and you do try @Error1, it wont catch it, right? I would be against this proposal, as I think beginners would see this as "this function throws two errors, and I can catch either of them".

It would with the original suggestion, not with either of the things I suggested.

I guess there's three things we might want:

1. fromException . toException == Just when applied to Either a b. We want this including when a ~ b to satisfy Add laws for the Exception class #135.
2. try @a $ throwIO (Left err :: Either a b) == Just err, and similar for @b / Right err. If a ~ b then we don't need to know whether a Left or Right was thrown.
3. try @(Either a b) $ throwIO (err :: a) == Just (Left err), and similar for err :: b / Right err. If a ~ b we don't care whether we get a Left or a Right.

I don't know if we can get (1) and (2) at the same time, except that if I understand @treeowl correctly we can use a type other than Either to forbid a ~ b. I think we can get (1) and (3) by doing something like "try to decode an Either a b; if that doesn't work try to decode an a; if that doesn't work try to decode a b". (Maybe this is going to get confusing if a Either a (Either b c) gets involved...?)

@Bodigrim, which of these did you want from the instance you were defining? Do you know which cabal-install wants? (Am I missing something from the list?)

[treeowl]

I think you get the gist of what I'm saying. A biased instance for Either a b feels unnatural to me, because Either is very explicitly a tagged union. I wouldn't personally support anything but the default definitions of toException and fromException for it. As I explained, I think it has two fairly reasonable choices for displayException.

[Bodigrim]

If code threw Either Error1 Error2, and you do try @Error1, it wont catch it, right? I would be against this proposal, as I think beginners would see this as "this function throws two errors, and I can catch either of them".

That's a good point.

Thanks all for the discussion. I think such instance would be less useful that I perceived originally. Let me withdraw the proposal.

[clyring]

If code threw Either Error1 Error2, and you do try @Error1, it wont catch it, right? I would be against this proposal, as I think beginners would see this as "this function throws two errors, and I can catch either of them".

For that purpose, I'd rather see an explicit exception union type. It could be a biased union

data BU a b = BL a | BL b

-- Catching BU a b will catch both `a` and `b`, preferring `a`. Throwing will unwrap.

Or it could be an explicitly disjoint union, probably most simply like this:

data DU a b where
  DLeft :: PlainEq a b ~ False => a -> DU a b
  DRight :: PlainEq a b ~ False => b -> DU a b

type family PlainEq a b where
  PlainEq a a = True
  PlainEq _ _ = False 

Here no bias is required because the types are definitely distinct. However, a little bit of bias can be convenient even in the disjoint case:

Making the two types explicitly distinct doesn't completely avoid the overlap issue because we may have a ~ SomeException. (Recall that fromException @SomeException = Just.)

Also, if we insist that forall e. fromException @e . toException @e = Just, that means in particular that (fromException . toException) (undefined :: Either a b) ought to equal Just (undefined :: Either a b). That seems to rule out any immediate pattern-matching in toException.