Skip to content
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

204 - no content with Maybe #208

Closed
bagl opened this issue Aug 31, 2015 · 18 comments
Closed

204 - no content with Maybe #208

bagl opened this issue Aug 31, 2015 · 18 comments

Comments

@bagl
Copy link

bagl commented Aug 31, 2015

Hello,
thanks for this great lib! It's such a joy to see the types doing great work in web apis 👍

I have one question. My endpoint upon POST should return data with 201 created or 204 no content. I model this using the Maybe.

Is there any other way to return 204 other than returning () as a result? Is extending Servant using new combinators the way to go?

@alpmestan
Copy link
Contributor

Maybe it's time to consider changing the current combinators... In another issue (or PR?), someone already suggested that we might want to make the status code sent with the response a type-level parameter, so that people can say "I want to return 200 if everything goes fine", etc. Then your suggestion amounts to being able to specify not just one status code but several, which hints at a list of status codes?

Regarding your last question, this is how I usually go about it: I write combinators that do what I want in the projects I need them for and if/when I'm satisfied with them I offer to include them in the next release. This means you always have an escape hatch to do things the way you want them done and can easily offer to share your work by creating a PR to include your combinators.

In this precise case though, I'm sincerely not sure what's the best way forward for the library to address your needs.

  • Modify the existing Get, Post, etc combinators, to specify the (or several) status code(s) returned if everything goes well, or
  • Create more general combinators for each method, where the current Get would be a special case of the more general one, with a fixed status code of 200 when things go well, or
  • Don't do anything in the library itself -- we can help you write some code to do what you want but we decide that need is too specific to be addressed out-of-the-box by the library itself, being afraid that it would make things too complex.

I'd probably go for the second one, but not sure yet. I'd love to hear people's thoughts on this.

@bagl
Copy link
Author

bagl commented Sep 3, 2015

There are cases where the exact successful status code can be determined only in runtime (usually after IO with DB) and this is supported for examle with the decision graph of Erlangs Webmachine.

In my case, it would be enough to handle Maybe a as a special case. () is handled as a special case too so I've tried to create a new router methodRouterMaybe and an instance AllCTRender ctypes a => HasServer (Post ctypes (Maybe a)) where but I get Conflicting family instance declarations:
ServerT (Post ctypes a) m
ServerT (Post ctypes (Maybe a)) m

Here, my beginner's haskell knowledge start's to go nowhere... Is this solvable in some easy to grasp way?

Bit of googling says that as () is concrete type so "overlap" is possible, but when there is some type parameter, it's not allowed. It seems this approach cannot work, then...

One more note, would you consider getting all the status codes that particular API endpoint can return (or specifically our handler) to be useful? Bundled with some documentation I would consider this a great help when implementing client for the API.

@alpmestan
Copy link
Contributor

Yeah, that overlapping instance is going to cause a lot of trouble. Because Maybe a can be considered as the a for the already existing instance.

Hence my comment in the previous reply: this requires either writing new combinators or modifying the existing ones. I'm not "arguing" about the benefits: with a comprehensive presentation of the status codes and responses that can be sent back to clients, we have a much greater deal of information which is all for the best: the API can be described very precisely by the API docs and client functions can understand much better what's going on when it doesn't get the expected "everything went well" response, because they know what status codes are expected.

I know @jkarni is interested in those ideas, however, this represents quite some work and I'm not sure we want to maintain a whole other set of combinators that let us describe APIs more comprehensively.

@jkarni
Copy link
Member

jkarni commented Sep 8, 2015

We talked about 204 for Nothing some time back, but it's a little less obviously okay than (), since Nothing does convey information. The only real (as in, general) solution I see is allowing absolutely explicit types (which is I think what you were saying in your last paragraph).

That's from the perspective of the library. For your specific use case, you can use the Left escape hatch, or write your own combinator.

@jkarni
Copy link
Member

jkarni commented Sep 9, 2015

Actually, how about a new datatype, isomorphic to Maybe? That way we don't accidentally surprise someone using Maybe, but still get the dynamic behaviour. Ideally we'd have a the whole stack still be a monad, so we'd really need MaybeT (or rather, the equivalent with a new datatype), but I'd have to think more about how we'd actually implement the latter.

@bagl
Copy link
Author

bagl commented Sep 10, 2015

I have used left as a temporary hack and will try to look into the newtype way as suggested. Thanks for help!

@kantp
Copy link
Contributor

kantp commented Sep 17, 2015

@alpmestan Since you're asking for people's thoughts: I think being able to explicitly set the status code would be very useful. For example, I'm currently trying to interact via a protocol that expects 200 for everything that went well (including POST). I could write a new method, say Post200, that behaves like Post but for the status code, but that feels ... not exactly elegant.

@jkarni What exactly is the left escape hatch? Do you just return a Left ServantErr with an errHTTPCode of 200? That would not allow me to send some actual data in the response though, would it?

@alpmestan
Copy link
Contributor

@kantp You know, we're always happy to receive any kind of feedback! And interestingly the problem you mention is one that's been on the back of my mind lately. I see two solutions, both of which are mentionned in this PR as well as in this thread.

  • Either put some StateT in there that lets you modify the response's status code (and maybe other things?) anywhere in your handler
  • Or offer "more generic" alternatives to Get, Post and friends (let's call them Get', Post' etc here) that take an additional parameter for the status code you want to return if everything goes well.

If we go for the second solution, then we would have:

type Get cts a = Get' cts a 200 -- the standard Get returns 200 if successful
type Post cts a = Post' cts a 201
-- etc

So that if you're not satisfied with the default status code, you can just use the general form and use another status code there. However this solution doesn't support having several "successful status codes", like the 201 or 204 case mentionned in the first comment of this issue.

Now, I'll be brutally honest. Those are not the solutions I actually want to go for. I'm starting to think EitherT ServantErr IO a is not the handler type we want. Ideally, we would have something like IO Response except that we the a -> Response conversion would be implicit, so that we could keep returning our haskell values from the handlers. And we would have a way to update the status code we want to return, the headers, etc. I sincerely don't believe, after spending a year using servant in anger, that making the response headers etc explicit is worth it.

With that said, I don't have any concrete idea for making this happen. Just throwing this out there so that I'm not brainstorming on this alone =)

@fizruk
Copy link
Member

fizruk commented Jan 20, 2016

@jkarni looks like this should be closed by #276. Please, confirm.

@jkarni
Copy link
Member

jkarni commented Jan 20, 2016

Yup. The solution we went with is explicitly declaring the status code, rather than doing ()/Maybe magic.

@jkarni jkarni closed this as completed Jan 20, 2016
@ublubu
Copy link

ublubu commented Jan 29, 2017

Does #276 really address this issue?

"My endpoint upon POST should return data with 201 created or 204 no content."

It seems to me that #276 lets you choose only one (status code, payload data type) per endpoint. I think this issue requires the ability to choose between multiple options like '[(201, some object), (204, no content)].

@newmana
Copy link

newmana commented Feb 7, 2017

My work around was to throw an custom error with 204 if there was no content i.e.

throwError $ ServantErr { errHTTPCode = 204
, errReasonPhrase = "No Content"
, errBody = ""
, errHeaders = []
}

@LeifW
Copy link
Contributor

LeifW commented Apr 12, 2018

Yeah, as people have mentioned above, the thing to do for e.g. a PUT is return 204 No Content if updating an existing resource, and 201 Created if the resource didn't already exist. Which is something determined at runtime and thus doesn't work to have in the type signature.
Should a new issue be opened for this?

@alpmestan
Copy link
Contributor

Yes, even though this is a problem we could solve with #841. cc @phadej :-)

@gwils
Copy link

gwils commented Jul 1, 2020

I'm hitting this in 2020 - it would be good to be able to choose the 2xy response from a type level list. '[201, 202]

@alpmestan
Copy link
Contributor

@gwils And 2020 has a solution :-) https://github.com/haskell-servant/servant-uverb - not quite the one you describe, but close enough I suppose.

@erewok
Copy link
Contributor

erewok commented Jul 1, 2020

@gwils You may be interested in this active PR, which adds UVerb to Servant (including a cookbook): #1314

@gwils
Copy link

gwils commented Jul 8, 2020

Thanks - there's a bright future ahead!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants