hedis icon indicating copy to clipboard operation
hedis copied to clipboard

More specific Error types

Open rehno-lindeque opened this issue 11 years ago • 4 comments

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)

rehno-lindeque avatar Mar 19 '14 15:03 rehno-lindeque

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: https://github.com/informatikr/hedis/pull/73

jsermeno avatar Apr 27 '16 15:04 jsermeno

The last idea for hedis errors was just throwing exceptions - https://github.com/informatikr/hedis/issues/54#issuecomment-191303479

qrilka avatar Apr 27 '16 19:04 qrilka

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.

jsermeno avatar Apr 27 '16 21:04 jsermeno

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?

qnikst avatar Jun 11 '23 18:06 qnikst