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

runSqlConn with ExceptT and friends #516

Closed
jkarni opened this Issue Dec 11, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@jkarni
Contributor

jkarni commented Dec 11, 2015

runSqlConn, when m is instantiated to a monad that doesn't necessarily execute all further actions (like ExceptT), may neither commit nor rollback (when its first argument is e.g. throwE someError). This seems like undesirable behaviour.

Original discussion here.

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Dec 12, 2015

Member

There's no valid instance of MonadBaseControl for ExceptT, so this should be impossible by the types we're using. Where does this instance come from, and what's its code? I've encountered this bug many times in the past in different guises.

Member

snoyberg commented Dec 12, 2015

There's no valid instance of MonadBaseControl for ExceptT, so this should be impossible by the types we're using. Where does this instance come from, and what's its code? I've encountered this bug many times in the past in different guises.

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Dec 12, 2015

Member

I see that the instance is in monad-control itself, which is surprising (I don't remember the short-circuiting transformers having such instances in the past). I'll look into this a bit more tomorrow.

Member

snoyberg commented Dec 12, 2015

I see that the instance is in monad-control itself, which is surprising (I don't remember the short-circuiting transformers having such instances in the past). I'll look into this a bit more tomorrow.

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Dec 13, 2015

Member

Can you test the code on master?

Member

snoyberg commented Dec 13, 2015

Can you test the code on master?

@ondrap

This comment has been minimized.

Show comment
Hide comment
@ondrap

ondrap Dec 14, 2015

Seems to work well, good work!

ondrap commented Dec 14, 2015

Seems to work well, good work!

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Dec 14, 2015

Member

Cool, thanks for confirming. New version on Hackage.

Member

snoyberg commented Dec 14, 2015

Cool, thanks for confirming. New version on Hackage.

@snoyberg snoyberg closed this Dec 14, 2015

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