Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not wrap exceptions when rethrowing #207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion freckle-exception/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
## [_Unreleased_](https://github.com/freckle/freckle-app/compare/freckle-exception-v0.0.0.0...main)
## [_Unreleased_](https://github.com/freckle/freckle-app/compare/freckle-exception-v0.1.0.0...main)

## [v0.1.0.0](https://github.com/freckle/freckle-app/compare/freckle-exception-v0.0.0.0...freckle-exception-v0.1.0.0)

Adjustment to the behavior of `catchJust`, `tryJust`, and `withException`:
Previously, if any of these utilities would catch and then rethrow an exception,
the exception would always be wrapped in `AnnotatedException` when rethrown.
Now any rethrown exceptions are rethrown exactly as they were caught.

The `HasCallStack` constraint has been removed from all of these functions, as they
now never construct or modify any exceptions and therefore have no reason to access
the call stack.

## [v0.0.0.0](https://github.com/freckle/freckle-app/tree/freckle-exception-v0.0.0.0/freckle-exception)

Expand Down
2 changes: 1 addition & 1 deletion freckle-exception/freckle-exception.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cabal-version: 1.18
-- see: https://github.com/sol/hpack

name: freckle-exception
version: 0.0.0.0
version: 0.1.0.0
synopsis: Some extensions to the annotated-exception library
description: Please see README.md
category: Exceptions
Expand Down
38 changes: 28 additions & 10 deletions freckle-exception/library/Freckle/App/Exception/MonadThrow.hs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,19 @@ catch action handler = withFrozenCallStack $ Annotated.catch action handler

catchJust
:: forall e b m a
. (Exception e, MonadCatch m, HasCallStack)
. (Exception e, MonadCatch m)
=> (e -> Maybe b)
-> m a
-> (b -> m a)
-> m a
catchJust test action handler =
withFrozenCallStack $ Annotated.catch action $ \e ->
maybe (Control.Monad.Catch.throwM e) handler (test e)
Control.Monad.Catch.catches
action
[ Control.Monad.Catch.Handler $ \caught ->
maybe (Control.Monad.Catch.throwM caught) handler (test caught)
, Control.Monad.Catch.Handler $ \caught@(AnnotatedException _ unwrapped) ->
maybe (Control.Monad.Catch.throwM caught) handler (test unwrapped)
]

catches
:: forall m a
Expand Down Expand Up @@ -97,25 +102,38 @@ try action = withFrozenCallStack $ Annotated.try action

tryJust
:: forall e b m a
. (Exception e, MonadCatch m, HasCallStack)
. (Exception e, MonadCatch m)
=> (e -> Maybe b)
-> m a
-- ^ Action to run
-> m (Either b a)
tryJust test action =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this have been written in terms of catchJust, so we only have to fix this bug once?

tryJust test action = catchJust test (Right <$> action) $ pure . Left

?

withFrozenCallStack $ Annotated.catch (Right <$> action) $ \e ->
maybe (Control.Monad.Catch.throwM e) (pure . Left) (test e)
withFrozenCallStack $
Control.Monad.Catch.catches
(Right <$> action)
[ Control.Monad.Catch.Handler $ \caught ->
maybe (Control.Monad.Catch.throwM caught) (pure . Left) (test caught)
, Control.Monad.Catch.Handler $ \caught@(AnnotatedException _ unwrapped) ->
maybe (Control.Monad.Catch.throwM caught) (pure . Left) (test unwrapped)
]

withException
:: forall e a m b
. (Exception e, MonadCatch m, HasCallStack)
. (Exception e, MonadCatch m)
=> m a
-> (e -> m b)
-> m a
withException action onException =
withFrozenCallStack $ Annotated.catch action $ \e -> do
void $ onException e
Control.Monad.Catch.throwM e
withFrozenCallStack $
Control.Monad.Catch.catches
action
[ Control.Monad.Catch.Handler $ \caught -> do
void $ onException caught
Control.Monad.Catch.throwM caught
, Control.Monad.Catch.Handler $ \caught@(AnnotatedException _ unwrapped) -> do
void $ onException unwrapped
Control.Monad.Catch.throwM caught
]

-- | When dealing with a library that does not use 'AnnotatedException',
-- apply this function to augment its exceptions with call stacks.
Expand Down
42 changes: 32 additions & 10 deletions freckle-exception/library/Freckle/App/Exception/MonadUnliftIO.hs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,23 @@ catch action handler = withFrozenCallStack $ Annotated.catch action handler

catchJust
:: forall e b m a
. (Exception e, MonadUnliftIO m, HasCallStack)
. (Exception e, MonadUnliftIO m)
=> (e -> Maybe b)
-> m a
-> (b -> m a)
-> m a
catchJust test action handler =
withFrozenCallStack $ Annotated.catch action $ \e ->
maybe (UnliftIO.Exception.throwIO e) handler (test e)
withFrozenCallStack $
UnliftIO.Exception.catches
action
[ UnliftIO.Exception.Handler $ \caught ->
maybe (UnliftIO.Exception.throwIO caught) handler (test caught)
, UnliftIO.Exception.Handler $ \caught@(AnnotatedException _ unwrapped) ->
maybe (UnliftIO.Exception.throwIO caught) handler (test unwrapped)
]

-- action $ \e ->
-- maybe (UnliftIO.Exception.throwIO e) handler (test e)
Comment on lines +80 to +81
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of leftovers.


catches
:: forall m a
Expand Down Expand Up @@ -98,25 +107,38 @@ try = withFrozenCallStack Annotated.try

tryJust
:: forall e b m a
. (Exception e, MonadUnliftIO m, HasCallStack)
. (Exception e, MonadUnliftIO m)
=> (e -> Maybe b)
-> m a
-- ^ Action to run
-> m (Either b a)
tryJust test action =
withFrozenCallStack $ Annotated.catch (Right <$> action) $ \e ->
maybe (UnliftIO.Exception.throwIO e) (pure . Left) (test e)
withFrozenCallStack $
UnliftIO.Exception.catches
(Right <$> action)
[ UnliftIO.Exception.Handler $ \caught ->
maybe (UnliftIO.Exception.throwIO caught) (pure . Left) (test caught)
, UnliftIO.Exception.Handler $ \caught@(AnnotatedException _ unwrapped) ->
maybe (UnliftIO.Exception.throwIO caught) (pure . Left) (test unwrapped)
]

withException
:: forall e a m b
. (Exception e, MonadUnliftIO m, HasCallStack)
. (Exception e, MonadUnliftIO m)
=> m a
-> (e -> m b)
-> m a
withException action onException =
withFrozenCallStack $ Annotated.catch action $ \e -> do
void $ onException e
UnliftIO.Exception.throwIO e
withFrozenCallStack $
UnliftIO.Exception.catches
action
[ UnliftIO.Exception.Handler $ \caught -> do
void $ onException caught
UnliftIO.Exception.throwIO caught
, UnliftIO.Exception.Handler $ \caught@(AnnotatedException _ unwrapped) -> do
void $ onException unwrapped
UnliftIO.Exception.throwIO caught
]

-- | When dealing with a library that does not use 'AnnotatedException',
-- apply this function to augment its exceptions with call stacks.
Expand Down
2 changes: 1 addition & 1 deletion freckle-exception/package.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: freckle-exception
version: 0.0.0.0
version: 0.1.0.0
maintainer: Freckle Education
category: Exceptions
github: freckle/freckle-app
Expand Down
Loading