jkarni
commented
7 years ago 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
Natcombined 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] SuccessResponseWe'd have
type API = Get '[JSON, HTML] ErrorResponse SuccessResponseWhere 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 -> StatusCodeWhich 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.
phadej
alpmestan
codedmart
lexi-lambda
cdepillabout
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:
If the client requests
/divide/6/3in JSON, then the result might look like this:If the client requests HTML, then the result might look like this:
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:Now I can add a case to my
ToJSONandToHTMLinstances to produce different errors upon failure, depending on the requested content type: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 ofExceptT. Sadly, if I produceServantErr, 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
Natto convert it toHandler. 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 returnHandler (StatusCode, a)instead ofHandler a. The downside to this, of course, is that it is remarkably non-typesafe, and it means you don’t get short-circuiting behavior ofExceptT.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: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.