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

More specific Error types #14

Open
rehno-lindeque opened this issue Mar 19, 2014 · 4 comments
Open

More specific Error types #14

rehno-lindeque opened this issue Mar 19, 2014 · 4 comments

Comments

@rehno-lindeque
Copy link

Currently for error conditions outside of transactions hedis returns a Left (Error ByteString :: Reply), which confuses me a little bit.

For example it was not immediately clear to me whether the error bytestring encodes only utf-8 strings. Further, it is only in the documentation where you can find that the Left reply will only ever contain an Error and not any of the SingleLine, Integer, Bulk or MultiBulk constructors.

Wouldn't it be possible to simplify those signatures to a more straight-forward Either Text a? Do I understand what's going on?

(PS attempting to quickly port https://github.com/scan/redissession from the deprecated redis to hedis)

@jsermeno
Copy link
Contributor

jsermeno commented Apr 27, 2016

Old issue, but perhaps this could be a place for discussing error handling in hedis? Any recommendations on how to best handle the cascading Either cases when dealing with many queries? Since Either is inside the Redis monad it appears difficult, or inconvenient to use existing Either monads, or monad transformers in the context of Redis.

One quick work around that I could think of, would be to export the data constructors for the Redis type. The data constructors could be exported directly from Redis.hs, or from Redis.Core.hs, which could be imported as a type of Internal module.

Change Redis() to Redis(..) in Redis/Core.hs, and make Redis.Core an exposed module.

That way, you could add code into your own codebase such as:

{-# LANGUAGE GeneralizedNewtypeDeriving, StandaloneDeriving, UndecidableInstances #-}
import Database.Redis
import Database.Redis.Core (Redis(..))

deriving instance MonadThrow Redis => MonadThrow Redis 

Then, you could have code that looked like:

runRedis c $ do
  maybeFoo <- get "foo" >>= toTM MyErr
  maybeBar <- get "bar" >>= toTM MyErr    

Where toTM throws your custom exception if a Left is found, and returns the value if a Right is found. You still have to deal with Maybe for some calls, but this makes sense to me, since you need to deal with whether a value exists or not.

Currently—with no changes—you could do something like:

runRedis c $ do
  maybeFoo <- toTM MyErr <$> get "foo"
  maybeBar <- toTM MyErr <$> get "bar"

But, using MonadThrow would also allow catching and throwing errors from within the Redis monad. In addition, hardly any changes are required. Thoughts?

In case the above sounds good: #73

@qrilka
Copy link
Contributor

qrilka commented Apr 27, 2016

The last idea for hedis errors was just throwing exceptions - #54 (comment)

@jsermeno
Copy link
Contributor

jsermeno commented Apr 27, 2016

I'm in favor of that, it would be a great change. Though, exposing the Redis data constructors could offer a work around until then.

In the long term, is there any reason why Redis wouldn't be turned into an instance of MonadThrow? I'd agree that in most cases, you probably aren't doing much error handling with the current Either, however it is still useful to be able to catch errors on a per-command basis.

@qnikst
Copy link
Collaborator

qnikst commented Jun 11, 2023

In our project we use the following approach:

runRedis $ runExceptT do
   result <- ExceptT $ someRedisFunction
   anotherResult <- ExceptT $ someOtherRedisFunction
   pure (f result anotherResult)

It fails if there was a error in any of the functions and returning Reply. The last thing is ugly but it's possible to write a helper for converting Reply to exception. This approach allows composing alternatives:

runRedis $ runExceptT do
   asum [ tryFirstAlternative
              , trySecondAlternative
              ]

And if exception arrives, entire computation will be stopped propagating an Exception outside. It also allows to use alternative transformers, for example one that allows to gather all errors (though we do not use such) that would be not so convenient with exceptions.

This approach requires only adding a helper (and possibly a runner instead of runExceptT).

What do you think about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants