Allow customizing the response code at the term level (at runtime) without sacrificing content negotiation #732

lexi-lambda · 2017-04-26T00:49:56Z

I was encouraged to open this issue based on a brief Twitter conversation with @alpmestan.

Currently, servant is super awesome as long as nothing goes wrong. I can write a high-level, declarative specification of what each of my API endpoints responds with, and I can keep all the rendering code separate from my logic. For example, imagine I have a simple API that looks like this:

newtype Quotient = Quotient Double
type API = "divide" :> Capture "x" Double :> Capture "y" Double :> Get '[JSON, HTML] Quotient

divide :: Double -> Double -> Handler Quotient
divide x y = return $ Quotient (x / y)

If the client requests /divide/6/3 in JSON, then the result might look like this:

{ "result": 2.0 }

If the client requests HTML, then the result might look like this:

<html>
  <head><title>Divide</title></head>
  <body><p>The result is: 2.0.</p></body>
</html>

This is great! But what if the user is naughty, and they try to request /divide/1/0? Obviously, I don’t want my server to blow up, so I tweak my handler to handle this case:

data Quotient
  = QuotientResult Double
  | DivisionByZero

divide :: Double -> Double -> Handler Quotient
divide _ 0 = return DivisionByZero
divide x y = return $ QuotientResult (x / y)

Now I can add a case to my ToJSON and ToHTML instances to produce different errors upon failure, depending on the requested content type:

{ "error": "DivisionByZero" }

<html>
  <head><title>Divide</title></head>
  <body><p>Error: you cannot divide by zero!</p></body>
</html>

However, I have a problem with this: my server is still responding with 200 OK, even though the response is obviously an error. It seems like my only option here is to respond with ServantErr, which makes sense, given that this is an error case, and I want the short-circuiting behavior of ExceptT. Sadly, if I produce ServantErr, I lose my nicely-formatted errors based on what content type the client requested.

In practice, I usually use a custom monad transformer stack for my handlers, and I use Nat to convert it to Handler. This doesn’t actually help at all, though, since I don’t have any access to the content type information at that point, so I can’t make the decision there.

There are probably a few ways to handle this. One way would be to add a different combinator, GetWithStatus, that allows the handler to return Handler (StatusCode, a) instead of Handler a. The downside to this, of course, is that it is remarkably non-typesafe, and it means you don’t get short-circuiting behavior of ExceptT.

A better solution would probably be to have a custom error type for an entire API, rather than ServantErr, which would be able to take advantage of content types to control how they are presented in the response. Here’s a completely imaginary interface for something like that:

data CalculatorError = Overflow | DivisionByZero

instance ToStatusCode CalculatorError where
  toStatusCode _ = 422

instance ToJSON CalculatorError where { ... }
instance ToHTML CalculatorError where { ... }

type API = UsingError CalculatorError :>
  ( "multiply" :> Capture "x" Double :> Capture "y" Double :> GetWithError '[JSON, HTML] Product
  , "divide" :> Capture "x" Double :> Capture "y" Double :> GetWithError '[JSON, HTML] Quotient
  )

multiply :: Double -> Double -> ExceptT CalculatorError Product
divide :: Double -> Double -> ExceptT CalculatorError Quotient

However, I’m not sure how feasible that is.

This seems related to #353 and #685, though this seems somewhat different, since it’s about application-specific errors rather than errors produced by servant. It’s probably the same issue as #296, though I’m not sure if things have changed much since then.

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

jkarni · 2017-04-26T10:03:50Z

I've thought about this a decent amount, though haven't really written much code for it. Here are two main options:

Simple : Accept header to error mapping

My opinion is that we should still have a single concrete type (like ServantErr) for errors, rather than try to allow the user to pick one and then somewhere (morally) existentialize it, since we already have Nat, which would allow picking different error types (as long as they're consistent throughout the server).

An obvious candidate is something like:

data NewServantErr = NewServantErr 
  {  acceptHeaderMapping :: Map AcceptHeader OldServantErr
  ,  defaultErr :: OldServantErr
}

We'd of course provide convenience functions around that. Combinators could use this same format, so it'd be related to #353 and #685 insofar as we probably want something that works in both cases.

This approach is simple, easy to implement, and isn't as large a change for end-users as the next one. The downside is that:

We still don't get any information about the type of the error statically
It'll be kind of annoying to write errors for each media type, and to be sure they correspond to one another, so people will likely turn to Nat combined with their own datatypes, replicating functionality that fits in quite naturally with the more ambitious approach.

Ambitious : Typed errors

The other option is to have information about the error type in the API type. So e.g. instead of:

type API = Get '[JSON] SuccessResponse

We'd have

type API = Get '[JSON, HTML] ErrorResponse SuccessResponse

Where ErrorResponse, like the SuccessResponse, is allowed to vary by endpoint. ErrorResponse is required to have the same instances (ToJSON, ToMarkup) as SuccessResponse; because content-type negotiation happens before the handlers are run, we know that this is sufficient. Additionally, we would probably have:

class IsError a where
   type PossibleStatusCodes :: [Nat]
   statusCodeOf :: a -> StatusCode

Which ErrorResponse would have to be an instance of.

Alternatively, instead of adding a type to the API type, we could start expecting the type of the handler response itself to have an instance of an IsError-like class, but I'm not sure I like that.

Even though servant would change quite a bit, it might be possible to make this nearly backwards-compatible with type aliases, which is reassuring.

I also don't think this one is too hard to implement, actually.

Note that I've somewhere advocated adding a lot more info to the API type. This is a sort of toned-down version of that.

In this approach, I'd say #353 and #685 are pretty much unrelated.

jkarni · 2017-04-26T10:06:06Z

@lexi-lambda the Using Calculation error is very similar to the second option above (sorry, should have mentioned that). I think in theory if we really wanted to we could have both forms (one being a type function for the other).

phadej · 2017-04-26T10:07:01Z

I like the ambitious approach. If we change Verb but have Get, Post etc as what they are that would be quite invisible change!

EDIT: we'll need to be able to create ErrorResponse from ServantErr.

alpmestan · 2017-04-26T10:09:19Z

I like the "ambitious" approach as well, it feels like it's a better direction to follow. The trickiest part I think would be to come up with the simplest API that would expose this new mechanism to users. I'm not saying we should aim for a perfect API right from the start, but rather "let's put some thoughts into it now and refine over time as we get feedback".

phadej · 2017-04-26T10:11:01Z

Could/Should we leave this a project for ZuriHac'17? (we could create a label, and tag potential tasks)?

alpmestan · 2017-04-26T10:13:47Z

That could be a nice task indeed. If someone wants to tackle this (or even just get started) before that would be very welcome by everyone though I think =)

codedmart · 2017-04-26T14:06:10Z

I also like the ambitious approach.

lexi-lambda · 2017-04-26T17:14:06Z

I agree with all the feedback here; the “ambitious” approach looks great. I’d consider working on implementing some of it myself, but I am not sure that I know nearly enough about how servant works to help much.

cdepillabout · 2017-04-27T10:59:57Z

I was talking to @alpmestan about this through email and he suggested I post about it here.

I've wanted something like this for a while now, and I am currently writing some code to solve a similar problem to this. Here's a work-in-progress:

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

I currently haven't written very much documentation, but you can get an idea of how it works by looking at this example:

https://github.com/cdepillabout/servant-checked-exceptions/blob/master/src/Servant/Checked/Exceptions.hs#L52

Also possibly the Envelope type:

https://github.com/cdepillabout/servant-checked-exceptions/blob/master/src/Servant/Checked/Exceptions/Internal/Envelope.hs#L101

My main goals with this were the following:

Represent errors in the API definition in an extensible way. This lets different APIs return a subset of the same errors in a type-safe way. Under the hood, it's using extensible sum-types.
Easily write documentation for the errors using servant-docs. I don't want to have to rewrite documentation for the same errors being returned by different handlers.

I feel like the current incarnation of the code has solved these two problems.

However, I still haven't figured out a good solution to the following problems:

I don't know of a good way to set the http status code for the error responses. It might be nice to have something like @jkarni's IsError type class above.
Ideally, I'd like the handlers to run in a short-circuiting monad, with the potential short-circuiting errors to be determined by the API type. Maybe there is a good way to implement this?
The HasServer instance for Throwing is somewhat nasty.

lexi-lambda · 2017-05-01T17:56:05Z

@cdepillabout, that’s really neat. The use of extensible sum types is really smart, and it seems like they fit in pretty naturally here. The lack of short circuiting is unfortunate, but it seems like that could be implemented using a different handler monad using Nat?

Do you think servant-checked-exceptions is in a place where I could give it a try? Even if it’s highly unstable, I’d still be interested in just playing with it a bit.

cdepillabout · 2017-05-02T15:51:56Z

@lexi-lambda I've released servant-checked-exceptions to hackage. It should be usable. You can find some examples of using it in the README.md in the github repo:

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

As far as short circuiting is concerned, I created an issue about it. I'm not sure of a good way to make it work (easily) with servant, but if you have any ideas I'd definitely be interested in hearing them.

phadej · 2017-06-08T10:34:12Z

ZuriHac note: This is advanced-expert task. See Ambitious : Typed errors in #732 (comment)

cdepillabout · 2018-02-13T12:36:21Z

I've updated the servant-checked-exceptions to be able to easily set the HTTP Status for errors.

A simple example would look like the following. This is similar to what @lexi-lambda and @jkarni were describing:

import Servant.Checked.Exceptions (Envelope, ErrStatus(..), Throws, pureSuccEnvelope, pureErrEnvelope)

data BadInputErr = BadInputErr

instance ErrStatus BadInputErr where
  toErrStatus :: BadInputErr -> Status
  toErrStatus _ = status400

data CapsErr = CapsErr

instance ErrStatus CapsErr where
  toErrStatus :: CapsErr -> Status
  toErrStatus _ = status404

type API = Capture "foobar" Text :> Throws BadInputErr :> Throws CapsErr :> Get '[JSON] Text

server :: ServerT API Handler
server = foobarEndpoint

foobarEndpoint :: Text -> Handler (Envelope '[BadInputErr, CapsErr] Text)
foobarEndpoint "hello" = pureSuccEnvelope "bye"
foobarEndpoint "HELLO" = pureErrEnvelope CapsErr
foobarEndpoint _ = pureErrEnvelope BadInputErr

app :: Application
app = serve (Proxy :: Proxy Api) server

I posted this in a previous comment, but to rehash on the benefits of using servant-checked-exceptions:

Errors for an endpoint are extensible. Different APIs can return a subset of the same errors in a type-safe way. It is easy to describe at the type level. It is easy to work with at the value level.
Writing documentation for errors is pretty easy with servant-docs.
Now with servant-checked-exceptions-1.0.0.0, it is possible to set different HTTP status codes for different exceptions.

The following problems still exist:

It would be nice to run in a short-circuiting monad, but still have errors be both type-safe and extensible. It feels like there should be a good way to do this, but I haven't figured it out.
The HasServer instance for Throwing is not necessarily as robust as it could be.

P.S. servant-checked-exceptions-1.0.0.0 currently only supports servant-0.11 (since that is what we are using at work), but it should be easy to update it to work with servant-0.12 and servant-0.13 as well.

alpmestan · 2018-02-13T13:22:08Z

@cdepillabout Would you perhaps feel like writing a cookbook entry for your library?

cdepillabout · 2018-02-13T13:32:03Z

@alpmestan Yes, I would be willing to do that. I added an issue to make sure I wouldn't forget.

However, I still haven't gotten the package documentation updated for this latest release, so that will have to happen first.

alpmestan · 2018-02-13T13:38:28Z

No problem :) Thanks in advance!

phadej added the ZuriHac17 label Apr 26, 2017

cdepillabout mentioned this issue May 2, 2017

set http status for errors cdepillabout/servant-checked-exceptions#2

Closed

lexi-lambda mentioned this issue May 2, 2017

Simplify the From/ToJSON instances for Envelope cdepillabout/servant-checked-exceptions#5

Open

phadej mentioned this issue May 14, 2017

Make mimeRender failable #644

Closed

phadej mentioned this issue Sep 10, 2017

Return JSON in throwError #802

Closed

phadej mentioned this issue Sep 21, 2017

Basic support for HTTP conditional requests (RFC 7232) #811

Open

cdepillabout mentioned this issue Feb 13, 2018

possibly write servant cookbook entry for servant-checked-exceptions cdepillabout/servant-checked-exceptions#21

Open

maksbotan mentioned this issue May 29, 2020

Configurable combinator errors #685

Closed

lfborjas mentioned this issue Sep 6, 2020

Customize error responses lfborjas/freenatalchart.xyz#15

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow customizing the response code at the term level (at runtime) without sacrificing content negotiation #732

Allow customizing the response code at the term level (at runtime) without sacrificing content negotiation #732

lexi-lambda commented Apr 26, 2017 •

edited

Loading

jkarni commented Apr 26, 2017

jkarni commented Apr 26, 2017

phadej commented Apr 26, 2017 •

edited

Loading

alpmestan commented Apr 26, 2017

phadej commented Apr 26, 2017

alpmestan commented Apr 26, 2017 •

edited

Loading

codedmart commented Apr 26, 2017

lexi-lambda commented Apr 26, 2017

cdepillabout commented Apr 27, 2017

lexi-lambda commented May 1, 2017

cdepillabout commented May 2, 2017

phadej commented Jun 8, 2017

cdepillabout commented Feb 13, 2018 •

edited

Loading

alpmestan commented Feb 13, 2018

cdepillabout commented Feb 13, 2018

alpmestan commented Feb 13, 2018

Allow customizing the response code at the term level (at runtime) without sacrificing content negotiation #732

Allow customizing the response code at the term level (at runtime) without sacrificing content negotiation #732

Comments

lexi-lambda commented Apr 26, 2017 • edited Loading

jkarni commented Apr 26, 2017

Simple : Accept header to error mapping

Ambitious : Typed errors

jkarni commented Apr 26, 2017

phadej commented Apr 26, 2017 • edited Loading

alpmestan commented Apr 26, 2017

phadej commented Apr 26, 2017

alpmestan commented Apr 26, 2017 • edited Loading

codedmart commented Apr 26, 2017

lexi-lambda commented Apr 26, 2017

cdepillabout commented Apr 27, 2017

lexi-lambda commented May 1, 2017

cdepillabout commented May 2, 2017

phadej commented Jun 8, 2017

cdepillabout commented Feb 13, 2018 • edited Loading

alpmestan commented Feb 13, 2018

cdepillabout commented Feb 13, 2018

alpmestan commented Feb 13, 2018

lexi-lambda commented Apr 26, 2017 •

edited

Loading

phadej commented Apr 26, 2017 •

edited

Loading

alpmestan commented Apr 26, 2017 •

edited

Loading

cdepillabout commented Feb 13, 2018 •

edited

Loading