Re-engineer Verb #841

phadej · 2017-10-23T05:50:34Z

The idea is that

Instead of having

a :> b :> Verb c

we could have

a :> b :> Verb (c :< d)

which is transformed into something like

a -> b -> m (c, d)

Verb glues RHS and LHS together. This way we can

Have checked exceptions (here "date" endpoint cannot error!)

type MyAPI = "date" :> Verb (Full Day)
        :<|> "time" :> Capture TimeZone :> Verb (ServantErr :< Full ZonedTime)

Get rid of {-# OVERLAPPING #-} instances, as in
```
type NoContentAPI = "tick" :> Verb NoContent
```
NoContent doesn't overlap Full a. Similarly we can have Stream a and
Headers ... :< ....

The devil in the details how to fit this into Router and DelayedIO
framework we have, but I don't see why it's not possible.

The names of auxiliary class HasServer' and Full combinator are open to
bikesheding.

Comments?

The text was updated successfully, but these errors were encountered:

alpmestan · 2017-10-23T08:24:21Z

A little note: Oleg and I have been discussing this over the past couple of days. We want to unify everything to avoid as much duplication as possible. The alternative to being able to "augment" the response information as done here would be to have all the possible "options" be type parameters to Verb. Here's what that would look like: https://gist.github.com/alpmestan/4ad03493cb6fe87e02b514418e9f7f02

So we really want @phadej's idea to work :) This would make many adhoc things non adhoc (stream would not have to be a totally separate hierarchy of response combinators -- cc @gbaz -- while responses that provide some headers would just augment the response with those instead of having the ugly Headers '[...] resp hack -- and it just goes on and on. We could similarly have "exhaustive endpoint descriptions" that describe the happy paths as well as the other ones, so that derived clients would basically be "more strongly typed" than they are now, because for non-happy paths you're stuck with a ServantError value. Redirects would also work pretty easily in this setting (they're just a combination of status code + "Location" header after all).

erewok · 2017-10-23T14:52:19Z

As a user of Servant and a drive-by commentator, I really like this proposal. (I'd probably bike shed the <: combinator, though.)

alpmestan · 2017-10-23T14:59:11Z

As a user of Servant and a drive-by commentator, I really like this proposal. (I'd probably bike shed the <: combinator, though.)

We want to hear from our users, maybe even more in this issue than usual. Feel free to suggest another operator and to illustrate with snippets of code you'd like to be able to write.

erewok · 2017-10-23T15:01:19Z

Thank you, @alpmestan. I should have probably kept my mouth shut until I could suggest something better. I think the reason I objected initially to <: is that my brain wants to coerce it into the opposite of :> whereas Verb ServantErr SomeReturnType looks like Either a b.

I also want to be clear that I think of this as bike-shedding: the overall improvement discussed here is excellent, in my opinion, so don't let me distract from that.

alpmestan · 2017-10-23T15:04:06Z

Well your point about your brain being disturbed by :< is a good one. In fact when @phadej initially suggested this approach, I think he used :+, which I interpreted to mean "we augment the response with more information".

erewok · 2017-10-23T15:04:53Z

One question:

Would it be possible to do something like Verb ('[HTML] ServantErr <: '[JSON] SomeReturnType)?

I would probably never do this, just wondering.

alpmestan · 2017-10-23T15:10:46Z

Being able to express different "response paths" is one of the important goals of this issue. response path == status code + headers + content types + type of the response body, here. Ideally we want full flexibility, so as to assume nothing about what users want to do. Because there's always someone who will want to do that very thing that you thought was irrelevant =) It makes sense to only return errors in JSON while making the "success" response available in JSON and HTML, in some apps, so it's not all that far fetched after all.

erewok · 2017-10-23T15:17:24Z

Well, in that case then, would it possible to have different code paths depending on, for instance, objects that get submitted? I imagine something like this:

type MyApi = "a" :> ReqBody '[JSON] SomeType :> BasicAuth "api-access" User :> Verb ('[HTML] MyAppErr401 <: '[JSON] MyAppErr404 <: '[JSON] SomeReturnType)

In other words, is it more like an HList than Either a b? Or is it more like Alternative, as in Error1 || SuccessOrError2 where SuccessOrError2 is Error2 || Success...

alpmestan · 2017-10-23T16:29:30Z

It's the sum-type equivalent of HList that would model things accurately here, indeed. We want to model the overall response of the endpoint as being one of several possibilities. It would be up to the endpoint's code, using some dedicated function(s), to return this or that kind of response depending on its inputs and anything else you want. But at least all the possible responses (including error responses) would be declared and all of this would be documented by servant-docs, clients would with your example give something like postSomeType :: SomeType -> User -> ClientM (Either SomeReturnType (Either MyAppErr401 MyAppErr404)), etc.

sboosali · 2017-11-15T00:13:50Z

the sum-type equivalent of HList

the vinyl records library has "co-records", which says that any value in some type can be any of a list of valid types. https://hackage.haskell.org/package/vinyl-0.6.0/docs/Data-Vinyl-CoRec.html

for example, CoRec Identity '[SomeReturnType, MyAppErr401, MyAppErr404].

with the type alias that implies that each value can have zero or more errors,

type Result es a = CoRec Identity (a ': es) 
type MyResult = Result SomeReturnType '[MyAppErr401, MyAppErr404]

a few other record libraries have n-ary variants, like the sum-of-products package, I mention vinyl because it's lightweight and the one I'm familiar with. and, since vinyl wraps each field with the same functor, working with effects is easier, since you can say stuff like Rec IO '[Integer, String] and conveniently convert it to IO (Rec Identity '[Integer, String]) with utility functions like rtraverse.

(sorry if this isn't relevant, I only skimmed the discussion)

arianvp · 2017-11-17T14:02:03Z

I have a little bit of prior work of this in my bachelor project https://github.com/arianvp/servis/raw/master/paper/paper.pdf

See the chapter about Explicit status Codes

arianvp · 2017-11-18T14:52:33Z

Brainstormed about this with @rightfold yesterday. How about something like this?:

Lets ditch the idea of having a coproduct of return types

You have Verb '[200, 201, 401] GET Signallying that an endpoint can return any of these three status codes. and some type family

type family IsElem x xs :: Constraint where
  IsElem x '[] = TypeError (Text "No")
  IsElem x (x ': xs) = ()
  IsElem x (y ': xs) = IsElem x xs

Then the way for people implementing a server to set a status code would be something like:

setStatusCode :: IsElem x xs => Proxy x -> Server (Verb Get xs) ()

This disallows a server implementing the spec from setting a status code that is not in the spec.

You could go one step further and use something like IxMonad to actually force the user to set the status code of the handler.

erewok · 2017-11-18T15:02:34Z

I like the direction that suggestion is going in, but (unless I'm misreading) it loses the option to specify different return types and different encodings for each return path.

To put those back in would look like:

Get '[(200, JSON, NormalType), (403, HTML, AuthError), etc.]

Edit:

Now that, I think about it, the original suggestion also had more of a combinator feel, so I could presumably do something like this:

type MyAppAuthErrors = AuthRequired401 :< AuthFailure403
Verb (MyAppAuthErrors :< Full ZonedTime)

alpmestan · 2017-11-18T15:45:17Z

@arianvp Well, we still need the different response types like @erewok said. They'd just be "annotations" on the status code. And this is all remarquably close to what we've been describing so far with @phadej above =)

arianvp · 2017-11-18T16:05:26Z

While we're at it, we can get rid of ServantErr right? Usually, you'd do something like err403 to give a different error code, but now that we can put the possible status codes in the type, a "Different status code" is not really an error scenario anymore, just a different code path. This would basically return our base type back to IO which is kind of neat

alpmestan · 2017-11-18T16:13:28Z

Yes, we want to offer that possibility. Not sure we want to get rid of ServantErr just yet. If we solve this, we can keep ServantErr around a little bit to allow people to smoothly move to the new approach. But we're not there yet anyway :)

jkarni · 2017-11-18T16:13:35Z

Content-types should be the same for all paths, since we have to resolve content type *before* running the code. (We've discussed this idea a few times in the past. I think there's already an issue for it that we should use, though I'm on my phone right now so will only check later)

…

On Saturday, November 18, 2017, Arian van Putten ***@***.***> wrote: While we're at it, we can get rid of ServantErr right? Usually, you'd do something like err403 to give a different error code, but now that we can put the possible status codes in the type, a "Different status code" is not really an error scenario anymore, just a different code path. This would basically return our base type back to IO which is kind of neat — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#841 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABlKmkX2XeGIbHfZJJNieXhNYP40PqI2ks5s3wBHgaJpZM4QCUq6> .

rightfold · 2017-11-18T16:15:33Z

I like the list of tuples idea. You can verify that the combination of status code and response type exists using a constraint.

respond :: IsResponse api s r => Proxy s -> r -> Server api

respond (Proxy :: Proxy '404) (CustomerNotFound uuid)

alpmestan · 2017-11-18T16:17:06Z

Note this is all very much the same idea, all throughout the thread, nothing new AFAICT. But the big challenge is to make it work @phadej -style (see first post) instead of exhaustive-style (see the gist that I linked to very early in this thread).

Changes Header, ReqBody and QueryParam to take a modifier list. First step to implement haskell-servant#856 Only adjust Links implementation. ResponseHeader story turns to be somewhat ugly, but it can be made elegant when haskell-servant#841 is implemnted, then we can omit HList aka Header Heterogenous List implementation.

Changes Header, ReqBody and QueryParam to take a modifier list. Resolves haskell-servant#856 ResponseHeader story turns to be somewhat ugly, but it can be made elegant when haskell-servant#841 is implemnted, then we can omit HList aka Header Heterogenous List implementation. - servant-server changes: Writing server side intepretations is quite simple using `unfoldRequestArgument`, which makes Header and QueryParam look quite the same. `ReqBody` cannot be easily made optional with current design (what that would mean: No Content-Type Header?), so that dimensions isn't used there. - Add HasLink for all the rest ComprehensiveAPI combinators - Add 'tricky' Header', QueryParam' endpoints to ComprehensiveAPI - servant-docs: Quick'n'dirty implementation. Don't use modifiers information (yet).

cdepillabout · 2018-04-04T09:29:07Z

@alpmestan pointed me to this issue in cdepillabout/servant-checked-exceptions#4 (comment).

It would be really nice if servant provided some way to encode what errors are returned by an API in a flexible manner. I'm using https://github.com/cdepillabout/servant-checked-exceptions to do it in a couple projects at work (and it is working pretty well), but it would be nice if Servant provided some sort of built-in support for this.

I think this proposal of re-enginnering Verb would enable this?

alpmestan · 2018-04-04T09:34:21Z

Yes, that's one of the goals. The big general goal is, now that we have a good idea of what people need from Verb, to make it support all these use cases without the need for ugly and fragile overlapping instances all over the place, but to instead support all those things right from the start.

The first candidate approach is one where we would just add extra annotations to specify that we add response headers, want to stream the response while executing the handler, declare strongly typed error cases (what your package does), etc. See @phadej's post at the very beginning of this thread.

The second one consists in making Verb parametrized by all those things, and to offer specialized combinators that would provide today's behaviour to try and not break things too hard, I suppose.

benweitzman · 2019-05-25T02:53:02Z

I'm pretty interested in seeing explicit errors land in servant.

Seems like these type of issues are being closed as dupes of this issue, so I'm just commenting here.

Here's a gist showing a way to add these to servant using a Throws e annotation: https://gist.github.com/benweitzman/5875d3e6a8cb1470cea5c32db3180f90

It's not a change to Verb, but it's pretty lightweight and backwards compatible.

Biggest downside I can see is mtl issues surrounding multiple targets of throwError, but I don't see this being avoidable in any context than throw either a ServantError or an application specific error.

Thoughts?

alpmestan · 2019-06-03T08:36:37Z

I think @cdepillabout has a package for this already (servant-checked-exceptions IIRC?). The work that @phadej has been doing and discussing here is really about enhancing Verb to support various features that are not possible today, out of the box.

Do let us know if @cdepillabout's package doesn't quite fit what you need, in which case it is I suppose relevant to this issue. :-)

cdepillabout · 2019-06-03T13:10:31Z

@benweitzman As alpmestan said, servant-checked-exceptions provides a Throws types you can use to describe the errors thrown by an endpoint. It uses a couple tricks to make Throws as easy to use as possible (although it makes it slightly more complicated than your gist):

https://github.com/cdepillabout/servant-checked-exceptions

Feel free to open an issue if you have any problems/suggestions.

fisx · 2019-07-30T21:58:51Z

... and another one, in a very early proof-of-concept stage:

https://github.com/wireapp/servant-uverb

servant-checked-exceptions seems more mature, but it's not flexible enough for what we need, so we explored the design space a little further.

alpmestan · 2019-07-31T06:24:44Z

Ha, very cool! I'll take a close look later today.

Changes Header, ReqBody and QueryParam to take a modifier list. Resolves haskell-servant/servant#856 ResponseHeader story turns to be somewhat ugly, but it can be made elegant when haskell-servant/servant#841 is implemnted, then we can omit HList aka Header Heterogenous List implementation. - servant-server changes: Writing server side intepretations is quite simple using `unfoldRequestArgument`, which makes Header and QueryParam look quite the same. `ReqBody` cannot be easily made optional with current design (what that would mean: No Content-Type Header?), so that dimensions isn't used there. - Add HasLink for all the rest ComprehensiveAPI combinators - Add 'tricky' Header', QueryParam' endpoints to ComprehensiveAPI - servant-docs: Quick'n'dirty implementation. Don't use modifiers information (yet).

2258: ADP-291: Document endpoints error codes in the API documentation r=hasufell a=hasufell # Issue Number ADP-291 # Implementation approach This is all best-effort. There are no static checks to ensure we didn't miss anything. There's ongoing work at servant to make this possible: haskell-servant/servant#841 But even that would require a major refactor. Testing all possible errors is also an option, but seems quite excessive. The workflow for figuring out the error codes is roughly: 1. follow the entry point to the the `Handler ()` function e.g. ```hs listTransactions :: forall ctx s t k n. (ctx ~ ApiLayer s t k) => ctx ... -> Handler [ApiTransaction n] ``` 2. check all `liftHandler` calls, which have functions with `ExceptT` as argument, e.g.: ```hs listTransactions :: forall ctx s k t. ( HasDBLayer s k ctx , HasNetworkLayer t ctx ) => ctx ... -> ExceptT ErrListTransactions IO [TransactionInfo] ``` 3. find all the LiftHandler instance, e.g. ```hs instance LiftHandler ErrListTransactions where handler = \case ErrListTransactionsNoSuchWallet e -> handler e ErrListTransactionsStartTimeLaterThanEndTime e -> handler e ErrListTransactionsMinWithdrawalWrong -> apiError err400 MinWithdrawalWrong "The minimum withdrawal value must be at least 1 Lovelace." ErrListTransactionsPastHorizonException e -> handler e ``` 4. follow the recursive handlers to resolve all errors and add client error types to `swagger.yaml`. The error code is the 3rd `ApiErrorCode` argument to `apiError`. 5. Some errors are implicit, e.g. failed parameter parsing etc. These are also in `ApiErrorCode`. Also see the special instance `instance LiftHandler (Request, ServerError)` 6. repeat until exhaustion # Overview - [x] Wallets - [x] Addresses - [x] Coin Selections - [x] Transactions - [x] Migrations - [x] Stake Pools - [x] Utils - [x] Network - [x] Proxy - [x] Settings - [ ] Byron-specific endpoints # Remarks 1. I did not double check the byron endpoints. Many of their endpoints share the same types in `swagger.yaml`, so I assumed they have the same behavior wrt error codes. 2. This will generally be hard to maintain, since yaml anchors aren't enough to express the overlaps of error codes and group them sensibly. It would need something like [dhall](https://github.com/dhall-lang/dhall-lang) to do that. 3. I removed 405 errors, because the HTTP method is part of the very spec. If you follow the spec, you can't get 405.  Co-authored-by: Julian Ospald <julian.ospald@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>

hasufell · 2020-11-01T12:44:09Z

Is there an introduction somewhere on how this new feature works?

cdepillabout · 2020-11-01T13:18:01Z

@hasufell It appears there is an entry in the cookbook for this, which has been added in #1314.

fisx · 2020-11-01T19:59:33Z

Is there an introduction somewhere on how this new feature works?

https://github.com/haskell-servant/servant/blob/c1105899f47d5537c1bdc8c86bee3523bdf05282/doc/cookbook/uverb/UVerb.lhs

2258: ADP-291: Document endpoints error codes in the API documentation r=hasufell a=hasufell # Issue Number ADP-291 # Implementation approach This is all best-effort. There are no static checks to ensure we didn't miss anything. There's ongoing work at servant to make this possible: haskell-servant/servant#841 But even that would require a major refactor. Testing all possible errors is also an option, but seems quite excessive. The workflow for figuring out the error codes is roughly: 1. follow the entry point to the the `Handler ()` function e.g. ```hs listTransactions :: forall ctx s t k n. (ctx ~ ApiLayer s t k) => ctx ... -> Handler [ApiTransaction n] ``` 2. check all `liftHandler` calls, which have functions with `ExceptT` as argument, e.g.: ```hs listTransactions :: forall ctx s k t. ( HasDBLayer s k ctx , HasNetworkLayer t ctx ) => ctx ... -> ExceptT ErrListTransactions IO [TransactionInfo] ``` 3. find all the LiftHandler instance, e.g. ```hs instance LiftHandler ErrListTransactions where handler = \case ErrListTransactionsNoSuchWallet e -> handler e ErrListTransactionsStartTimeLaterThanEndTime e -> handler e ErrListTransactionsMinWithdrawalWrong -> apiError err400 MinWithdrawalWrong "The minimum withdrawal value must be at least 1 Lovelace." ErrListTransactionsPastHorizonException e -> handler e ``` 4. follow the recursive handlers to resolve all errors and add client error types to `swagger.yaml`. The error code is the 3rd `ApiErrorCode` argument to `apiError`. 5. Some errors are implicit, e.g. failed parameter parsing etc. These are also in `ApiErrorCode`. Also see the special instance `instance LiftHandler (Request, ServerError)` 6. repeat until exhaustion # Overview - [x] Wallets - [x] Addresses - [x] Coin Selections - [x] Transactions - [x] Migrations - [x] Stake Pools - [x] Utils - [x] Network - [x] Proxy - [x] Settings - [ ] Byron-specific endpoints # Remarks 1. I did not double check the byron endpoints. Many of their endpoints share the same types in `swagger.yaml`, so I assumed they have the same behavior wrt error codes. 2. This will generally be hard to maintain, since yaml anchors aren't enough to express the overlaps of error codes and group them sensibly. It would need something like [dhall](https://github.com/dhall-lang/dhall-lang) to do that. 3. I removed 405 errors, because the HTTP method is part of the very spec. If you follow the spec, you can't get 405.  Co-authored-by: Julian Ospald <julian.ospald@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>

phadej added the rfc label Oct 23, 2017

alpmestan mentioned this issue Oct 26, 2017

Redirect combinator ? #117

Open

alpmestan mentioned this issue Jan 17, 2018

Have a streaming request body for servant-client #886

Closed

alpmestan mentioned this issue Mar 4, 2018

Nothing is translated to null #914

Closed

alpmestan mentioned this issue Apr 4, 2018

additional instances of HasServer and HasClient for Throwing in apis that set Headers cdepillabout/servant-checked-exceptions#4

Open

alpmestan mentioned this issue Apr 13, 2018

204 - no content with Maybe #208

Closed

This was referenced Jun 24, 2018

Stream result is broken. #994

Closed

Post-841: Split Stream #995

Open

Streaming refactoring #991

Merged

domenkozar mentioned this issue Jul 6, 2018

Allow endpoints to advertise returnable errors #349

Closed

phadej mentioned this issue Aug 12, 2018

How to handle impure exceptions in servant-server? #1022

Open

alpmestan mentioned this issue Dec 28, 2018

Summer of Code Haskell Submissions? #1102

Closed

alpmestan mentioned this issue Jan 31, 2019

Cookie/SetCookie combinators - proper cookies support #468

Closed

phadej mentioned this issue Feb 3, 2019

Catch ServantErr exceptions in liftIO definition for Handler? #1111

Closed

mheinzel mentioned this issue Dec 12, 2019

servant-client doesn't allow HasClient instances to handle non-2xx responses #1253

Open

fisx added the zurihac2020 https://github.com/haskell-servant/servant/issues/1299 label May 18, 2020

fisx mentioned this issue May 18, 2020

zurihac2020 #1299

Closed

fisx mentioned this issue Jun 16, 2020

union verbs #1314

Merged

16 tasks

hasufell mentioned this issue Oct 19, 2020

ADP-291: Document endpoints error codes in the API documentation cardano-foundation/cardano-wallet#2258

Merged

11 tasks

fisx closed this as completed in #1314 Oct 31, 2020

alpmestan mentioned this issue Jun 11, 2022

[RFC] MultiVerb #1454

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-engineer Verb #841

Re-engineer Verb #841

phadej commented Oct 23, 2017 •

edited

Loading

alpmestan commented Oct 23, 2017 •

edited

Loading

erewok commented Oct 23, 2017 •

edited

Loading

alpmestan commented Oct 23, 2017

erewok commented Oct 23, 2017 •

edited

Loading

alpmestan commented Oct 23, 2017

erewok commented Oct 23, 2017 •

edited

Loading

alpmestan commented Oct 23, 2017

erewok commented Oct 23, 2017 •

edited

Loading

alpmestan commented Oct 23, 2017

sboosali commented Nov 15, 2017

arianvp commented Nov 17, 2017 •

edited

Loading

arianvp commented Nov 18, 2017

erewok commented Nov 18, 2017 •

edited

Loading

alpmestan commented Nov 18, 2017

arianvp commented Nov 18, 2017

alpmestan commented Nov 18, 2017

jkarni commented Nov 18, 2017 via email

rightfold commented Nov 18, 2017

alpmestan commented Nov 18, 2017 •

edited

Loading

cdepillabout commented Apr 4, 2018

alpmestan commented Apr 4, 2018

benweitzman commented May 25, 2019

alpmestan commented Jun 3, 2019

cdepillabout commented Jun 3, 2019 •

edited

Loading

fisx commented Jul 30, 2019

alpmestan commented Jul 31, 2019

hasufell commented Nov 1, 2020

cdepillabout commented Nov 1, 2020

fisx commented Nov 1, 2020

Re-engineer Verb #841

Re-engineer Verb #841

Comments

phadej commented Oct 23, 2017 • edited Loading

alpmestan commented Oct 23, 2017 • edited Loading

erewok commented Oct 23, 2017 • edited Loading

alpmestan commented Oct 23, 2017

erewok commented Oct 23, 2017 • edited Loading

alpmestan commented Oct 23, 2017

erewok commented Oct 23, 2017 • edited Loading

alpmestan commented Oct 23, 2017

erewok commented Oct 23, 2017 • edited Loading

alpmestan commented Oct 23, 2017

sboosali commented Nov 15, 2017

arianvp commented Nov 17, 2017 • edited Loading

arianvp commented Nov 18, 2017

erewok commented Nov 18, 2017 • edited Loading

alpmestan commented Nov 18, 2017

arianvp commented Nov 18, 2017

alpmestan commented Nov 18, 2017

jkarni commented Nov 18, 2017 via email

rightfold commented Nov 18, 2017

alpmestan commented Nov 18, 2017 • edited Loading

cdepillabout commented Apr 4, 2018

alpmestan commented Apr 4, 2018

benweitzman commented May 25, 2019

alpmestan commented Jun 3, 2019

cdepillabout commented Jun 3, 2019 • edited Loading

fisx commented Jul 30, 2019

alpmestan commented Jul 31, 2019

hasufell commented Nov 1, 2020

cdepillabout commented Nov 1, 2020

fisx commented Nov 1, 2020

phadej commented Oct 23, 2017 •

edited

Loading

alpmestan commented Oct 23, 2017 •

edited

Loading

erewok commented Oct 23, 2017 •

edited

Loading

erewok commented Oct 23, 2017 •

edited

Loading

erewok commented Oct 23, 2017 •

edited

Loading

erewok commented Oct 23, 2017 •

edited

Loading

arianvp commented Nov 17, 2017 •

edited

Loading

erewok commented Nov 18, 2017 •

edited

Loading

alpmestan commented Nov 18, 2017 •

edited

Loading

cdepillabout commented Jun 3, 2019 •

edited

Loading