haskus / packages

Haskus packages
https://haskus.org/
24 stars 11 forks source link

Excepts ergonomics #32

Closed hasufell closed 2 years ago

hasufell commented 4 years ago

I'm envisioning an exception system where I rethrow low-level exceptions as high-level exceptions, while still encapsulating the underlying error. An example:

    ------------------------
    --[ Low-level errors ]--
    ------------------------

data HTTPStatusError = HTTPStatusError Int
  deriving Show

data NoLocationHeader = NoLocationHeader
  deriving Show

data TooManyRedirs = TooManyRedirs
  deriving Show

    -------------------------
    --[ High-level errors ]--
    -------------------------

-- | A download failed. The underlying error is encapsulated.
data DownloadFailed = forall es . Show (V es) => DownloadFailed (V es)

deriving instance Show DownloadFailed

    -------------------------------
    --[ Error handling funtions ]--
    -------------------------------

-- https://hackage.haskell.org/package/haskus-utils-variant-3.0/docs/Haskus-Utils-Variant-Excepts.html

reThrowAll :: forall e es es' a m
            . (Monad m, e :< es')
           => (V es -> e)
           -> Excepts es m a
           -> Excepts es' m a
reThrowAll f = catchAllE (throwE . f)

reThrowAllIO :: forall e es es' a m
              . ( MonadCatch m
                , Monad m
                , MonadIO m
                , e :< es'
                , es ~ '[IOException]
                )
             => (V es -> e)
             -> Excepts es m a
             -> Excepts es' m a
reThrowAllIO f =
  handleIO (throwE . f . variantFromValue) . catchAllE (throwE . f)

There are currently two problems with this approach:

  1. You never know whether you have extraneous error types in your open variant:
data Foo = Foo
  deriving Show

data Bar = Bar
  deriving Show

foo :: Monad m => Excepts '[Bar , Foo] m ()
foo = do
  throwE Bar
  pure ()

This makes it impossible to know which errors are "lies" after a refactor.

  1. You never know whether you catched all exceptions of the inner Monad.

Any ideas?

hsyl20 commented 4 years ago

For the first problem, the issue is that the Monad interface isn't expressive enough: currently (>>=) :: m a -> (a -> m b) -> mb but we would like (>>=) :: E e1 a -> (a -> E e2 b) -> E (F e1 e2) b. It should be possible to work something with rebindable syntax (and to wait for https://github.com/ghc-proposals/ghc-proposals/pull/216 to make it more practical to use)

For the second problem, you mean real exception like IOException?

hasufell commented 4 years ago

For the second problem, you mean real exception like IOException?

Yes. I can catch them all and turn them into an error type, but it's never visible in the types.

hsyl20 commented 3 years ago

For (1), I hope to find some time to try with qualified-do now that 9.0 is out.

For (2), isn't it that your definition of DownloadFailed in your example with the forall is too permissive? If you know the errors that will be encapsulated by DownloadFailed beforehand, you can use splitVariant to encapsulate them into DownloadFailed and rethrow only the unhandled ones.

For IOException, I fear you will have to import GHC.IO.Exception and somehow lift the IOErrorType to the type-level to integrate it with Excepts.

hasufell commented 2 years ago

Another thing I have no solution to is how to deal with tuples, when e.g. using sequenceE in ghcup I have this peculiar situation to return a tuple, e.g.:

            VLeft (V (FileAlreadyExistsError fp, ())) -> do
              runLogger $ logWarn $
                "File " <> T.pack fp <> " already exists. Use 'ghcup install cabal --isolate " <> T.pack fp <> " --force ..." <> "' if you want to overwrite."
              pure $ ExitFailure 3
            VLeft e -> do
              runLogger $ do
                logError $ T.pack $ prettyShow e
                logError $ "Also check the logs in " <> T.pack logsDir
              pure $ ExitFailure 4

Now I would like a case to match on any tuple, like VLeft (V (e, ())... but I'm unable to make that work. It always requires either VLeft e or specifying the values inside the tuple.

hsyl20 commented 2 years ago

It's usually difficult to work with generic types into variants. I think you could work something with a type-class matching these tuples and reduceVariant for example. But this solution seems too heavy for your use case.

I don't understand why you use sequenceE and hence build a product in the first place. Is it only to run setCabal even if installCabal failed? If it's the case, it could be simpler to do this explicitly:

  v_install <- runE (installCabal...)
  v_set     <- runE (setCabal ...)

  -- bundle both exceptions into one?
  case (v_install,v_set) of
    (VLeft e1, VLeft e2) -> throwE (InstallSetError e1 e2)
    (VLeft e , _       ) -> throwE e
    (_       , VLeft e ) -> throwE e
    (VRight a, VRight b) -> pure (a,b)

  -- drop second exception?
   case (v_install,v_set) of
    (VLeft e , _       ) -> throwE e
    (_       , VLeft e ) -> throwE e
    (VRight a, VRight b) -> pure (a,b)
hasufell commented 2 years ago

It's usually difficult to work with generic types into variants. I think you could work something with a type-class matching these tuples and reduceVariant for example. But this solution seems too heavy for your use case.

I don't understand why you use sequenceE and hence build a product in the first place. Is it only to run setCabal even if installCabal failed? If it's the case, it could be simpler to do this explicitly:

  v_install <- runE (installCabal...)
  v_set     <- runE (setCabal ...)

  -- bundle both exceptions into one?
  case (v_install,v_set) of
    (VLeft e1, VLeft e2) -> throwE (InstallSetError e1 e2)
    (VLeft e , _       ) -> throwE e
    (_       , VLeft e ) -> throwE e
    (VRight a, VRight b) -> pure (a,b)

  -- drop second exception?
   case (v_install,v_set) of
    (VLeft e , _       ) -> throwE e
    (_       , VLeft e ) -> throwE e
    (VRight a, VRight b) -> pure (a,b)

That doesn't seem to typecheck:

    • V '[NotInstalled] not found in list:
       '[AlreadyInstalled, ArchiveResult, BuildFailed, CopyError,
         DigestError, DirNotEmpty, DownloadFailed, FileAlreadyExistsError,
         FileDoesNotExistError, GPGError, MergeFileTreeError,
         NextVerNotFound, NoDownload, NoToolVersionSet, NotInstalled,
         ProcessError, TagNotFound, TarDirDoesNotExist,
         UninstallFailed, UnknownArchive]
hsyl20 commented 2 years ago

That doesn't seem to typecheck:

Ah yes, my bad. I guess we need something like the following to replace throwE:

-- | Throw some exception
throwSomeE :: forall es' es a m. (Monad m, LiftVariant es' es) => V es' -> Excepts es m a
{-# INLINABLE throwSomeE #-}
throwSomeE = Excepts . pure . VLeft . liftVariant
hasufell commented 2 years ago

Excellent. That seems to work. I'd say we should add throwSomeE then.

hsyl20 commented 2 years ago

Done in https://github.com/haskus/packages/commit/97eb0eaebb306f230f0abd72017550cd5bf5399f