finally and bracket don't work with ExceptT

[delfigamer]

safe-exceptions/src/Control/Exception/Safe.hs

Lines 428 to 449 in 9d8baf8

	-- | Async safe version of 'E.bracket' with access to the exception in the
	-- cleanup action.
	--
	-- @since 0.1.7.0
	bracketWithError :: forall m a b c. C.MonadMask m
	=> m a -> (Maybe SomeException -> a -> m b) -> (a -> m c) -> m c
	bracketWithError before after thing = C.mask $ \restore -> do
	x <- before
	res1 <- C.try $ restore (thing x)
	case res1 of
	Left (e1 :: SomeException) -> do
	-- explicitly ignore exceptions from after. We know that
	-- no async exceptions were thrown there, so therefore
	-- the stronger exception must come from thing
	--
	-- https://github.com/fpco/safe-exceptions/issues/2
	_ :: Either SomeException b <-
	C.try $ C.uninterruptibleMask_ $ after (Just e1) x
	C.throwM e1
	Right y -> do
	_ <- C.uninterruptibleMask_ $ after Nothing x
	return y

If the underlying monad is ExceptT, and the interior action performs throwError, then the release action gets skipped.

This can be checked with this test:

    describe "bracketWithError" $ do
      it "should lift through ExceptT properly" $ do
        ref <- newIORef ("no resource" :: Text)
        _ <-
          runExceptT $
            Control.Exception.Safe.bracketWithError
              ( do
                  lift $ readIORef ref `shouldReturn` "no resource"
                  lift $ writeIORef ref "resource acquired"
              )
              ( \_ () -> do
                  lift $ readIORef ref `shouldReturn` "resource used"
                  lift $ writeIORef ref "resource released"
              )
              (\() -> do
                  lift $ readIORef ref `shouldReturn` "resource acquired"
                  lift $ writeIORef ref "resource used"
                  throwError ControlledException
              )
          :: IO (Either ControlledException Void)
        readIORef ref `shouldReturn` "resource released"

With the current version (0.1.7.1), this test fails with expected: "resource released", but got: "resource used".

Since finally and bracket are written as special cases of bracketOnError, they get affected by this bug as well.

As this library is using exceptions's monad typeclasses, I think this function should be re-written in terms of generalBracket, so that it would handle the ExceptT transformer too.

[snoyberg]

Seems like a reasonable change, interested in sending a PR?