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

Fixes #23, second attempt :) #32

Merged
merged 1 commit into from Jul 18, 2013

Conversation

Projects
None yet
2 participants
@adinapoli

adinapoli commented Jul 18, 2013

Ok, it should be almost there.

The only warty thing is, as you might notice, is that we call error when we fail. I've tried to stick to the project convention , doing something like:

case code -> throwError $ HsenvException $ bla bla bla

But I got an error which puzzles me:

src/Actions.hs:326:15:
    Couldn't match type `GHC.IO.Exception.IOException'
                  with `HsenvException'
    When using functional dependencies to combine
      Control.Monad.Error.Class.MonadError
        GHC.IO.Exception.IOException IO,
        arising from the dependency `m -> e'
        in the instance declaration in `Control.Monad.Error.Class'
      Control.Monad.Error.Class.MonadError HsenvException IO,
        arising from a use of `throwError' at src/Actions.hs:326:15-24
    In the expression: throwError
    In the expression:
      throwError
      $ HsenvException
        $ "Failed to download "
          ++ name ++ ": http response returned " ++ show code

I suspect it's because we are lifting the whole computation from the IO, but I might be wrong. Insight and fixes are welcome. One quick to do would be to use throw instead of throwError, where the former lives inside Control.Exception. To do that, we also need to add an instance of Exception to HsenvException, which is a one liner:

instance Exception HsenvException

A.

@tmhedberg tmhedberg merged commit 9399fbf into tmhedberg:master Jul 18, 2013

@tmhedberg

This comment has been minimized.

Show comment
Hide comment
@tmhedberg

tmhedberg Jul 18, 2013

Owner

Wonderful! Thanks for being accepting of my fussiness. :) This looks much better now.

I dealt with the throwError issue you mentioned by tweaking the code slightly. You're spot on about the cause of the issue. The fundep on mtl's MonadError class makes it so that throwError in IO must throw an IOException, rather than some other exception type. I changed it so that the lifted I/O computation now returns a Maybe HsenvException, and once the I/O completes and we're back in the Hsenv monad, I throwError from there, if there is an exception to be thrown.

Owner

tmhedberg commented Jul 18, 2013

Wonderful! Thanks for being accepting of my fussiness. :) This looks much better now.

I dealt with the throwError issue you mentioned by tweaking the code slightly. You're spot on about the cause of the issue. The fundep on mtl's MonadError class makes it so that throwError in IO must throw an IOException, rather than some other exception type. I changed it so that the lifted I/O computation now returns a Maybe HsenvException, and once the I/O completes and we're back in the Hsenv monad, I throwError from there, if there is an exception to be thrown.

@adinapoli

This comment has been minimized.

Show comment
Hide comment
@adinapoli

adinapoli Jul 18, 2013

Wonderful, thanks a lot! :)

adinapoli commented Jul 18, 2013

Wonderful, thanks a lot! :)

@adinapoli adinapoli deleted the adinapoli:issue-23 branch Jul 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment