-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
union verbs #1314
union verbs #1314
Conversation
Hi. I would like to hear your opinion about my idea. Since newtype UVerbT xs m a = UVerbT { unUVerbT :: ExceptT (Union xs) m a }
deriving newtype (Functor, Applicative, Monad, MonadTrans)
-- | Deliberately hide 'ExceptT's 'MonadError' instance to be able to use
-- underlying monad's instance.
instance MonadError e m => MonadError e (UVerbT xs m) where
throwError = lift . throwError
catchError (UVerbT act) h = UVerbT $ ExceptT $
runExceptT act `catchError` (runExceptT . unUVerbT . h)
-- | This combinator runs 'UVerbT'. It applies 'respond' internally, so the handler
-- may use the usual 'return'.
runUVerbT :: (Monad m, HasStatus x, IsMember x xs) => UVerbT xs m x -> m (Union xs)
runUVerbT (UVerbT act) = either id id <$> runExceptT (act >>= respond)
-- | Short-circuit 'UVerbT' computation returning one of the response types.
throwUVerb :: (Monad m, HasStatus x, IsMember x xs) => x -> UVerbT xs m a
throwUVerb = UVerbT . ExceptT . fmap Left . respond Example usage: h :: Handler (Union '[Foo, WithStatus 400 Bar])
h = runUVerbT $
when (something bad) $
throwUVerb $ WithStatus @400 Bar
when (really bad) $
throwError $ err500
-- a lot of code here...
return $ Foo 1 2 3 |
@maksbotan I like that. I have to admit that I always get really confused with errors: do I want Still, I don't tend to think of returning a 400 as "throwing" anything. I handle bad requests all the time and the server is operating fine, so in my mind these are not exceptions. With this in mind, I actually would not recommend making it seem like it's This is a drive-by opinion, though, by someone who's never bothered to learn all the error/exception stuff in Haskell, so feel free to dismiss. |
Well, I made this for the case when you are doing some validation in your handlers, and let's say there are a lot of cases you need to check and all of them result in some kind of Anyway, naming is a very subjective matter. I do not have strong preference for |
@maksbotan I also like your idea, but I would (slightly) prefer to move it to a separate PR, in the hope this will get merged faster. Do you have an opinion? |
Hm... Shouldn't |
Sure, I wasn't suggesting including it into this PR :) It's just that I was experimenting with Re |
Having an ExceptT-wrapper for this sounds neat. I wouldn't want to integrate it into the uverb-mechanism directly though as I personally want to get away from the idea that "status codes" are "exceptional" and should be "short circuiting" as i do not think that is the case in general. It should be something a person can opt in to by wrapping in a But I totally agree having the option to short-circuit is extremely convenient. Scotty for example has |
Note that |
Yep completely agreed. I just wonder if servant is the right place for this .. This feels like a sort of type that must be available somewhere in some library already? If it's not; I wonder why As said; I see some eer-y parallels with extensible effects libraries I think that e.g. with the https://hackage.haskell.org/package/extensible-effects-5.0.0.1/docs/Control-Eff-Exception.html
Or in I'm not sure if we should hook into a specific effects library for this; as servant has always been MTL-based so far... but it feels odd to reinvent this behaviour if it's somewhere else already |
I'm sorry for stirring the confusion! |
Don't feel sorry! It sounds like a super useful pattern. It is at the least worth mentioning in whatever tutorial we come up for with this, |
I took the liberty of adding it to the cookbook and making you the author of the commit: 084941a I hope that was ok? Thanks again for dumping the idea here, I think that was very helpful! :) (@arianvp feel free to add your thoughts on which libraries could be used for that.) |
Thanks @fisx! |
By the way, what's the status of this one? |
it's all in the description. :) i'll try to talk to @arianvp about this today, perhaps we can merge it as well, and then make the ultimate release. |
Also remove the unused DerivingVia that broke earlier ghcs.
Hi @fisx! Is there any progress on this? I would love to see UVerbs in servant master :) |
This was poorly named: @as `Contains` bs@ states whether that as *is contained in* bs. But more importantly it was dead code.
Trying to build with cabal now, and I also have no luck. With cabal 3.2.0.0, ghc 8.8.4, I get a number of entertaining errors, but this is my favorite:
|
That's strange... Maybe you could try nuking your cabal store? As far as I see, it built successfully on Travis. And I have no problems building with Cabal (via haskell.nix) locally. |
Yes, I wanted to avoid that, but it's probably a good idea...
Ah... nix! :) that probably helps, yes. @arianvp can you still reproduce the problem on your machine? |
Nice work, @fisk! |
This is amazing, thanks!! Error handling was a large gap in servant before this landed. |
Fixes #841
TODO:
DerivingVia
("old" ghcs don't have it).Servant.Client.UVerb
go toservant-client-core
?TODO elsewhere, outside this PR:
RunStreamingClient
,ClientF
(addacceptStatus
argument like forRunClient
class)