HTTP status codes - Githubissues

cognitect-labs / vase

Data driven microservices

Eclipse Public License 1.0

374 stars 40 forks source link

HTTP status codes #30

Closed dazld closed 7 years ago

dazld commented 7 years ago

Not a bug as such, but seems that Vase should follow HTTP conventions for status codes - especially for following cases:

Nothing found should be a 404 (currently 200) on routes that include a resource identifier in the path
Bad requests should be a 400 (currently 500 and exposing stack trace to client)

ohpauleez commented 7 years ago

Hi @dazld -- thanks for reaching out!

Bad requests definitely should be 400. If you've found a case where a bad request causes a 500, please open up an issue so I can patch it up. You might be seeing the stack trace because you have the "dev" interceptors turned active.

I typically interpret 404 as the entire resource does not exist. An empty result would be the resource exists, it was found, there's just nothing there. Does that make sense or do you typically think of empty resources as also Not Found?

dazld commented 7 years ago

good tip - I'll try without dev. however - even if this does then return a bad request, I think it's worth considering whether this should be so different across the two modes. my preference would be for status codes to be 1:1 the same across configs.

about the 200/404 - I was looking at the pet store example. for the general collections endpoint (/pets) - this should clearly be a 200 even if empty, as you mention. However, for the sub resources (eg: /pets/foo), if this doesn't exist, returning a 200 here was confusing.

ohpauleez commented 7 years ago

Sorry for not being clearer -- the statuses will be the same, but you won't see a stack trace in body in "prod" mode. That said, you should still report what caused the 500, because all bad requests should be 400 (you've found a case we've missed in testing).

Ah, I see what you mean about resources and resource lookup. The Petstore example is from an earlier version of Vase, when we had temporarily removed response generation. You can control status code and automatic response generation using the response utilities. I'll make sure we include better examples of using those to shape response/status codes in upcoming samples.

ohpauleez commented 7 years ago

@dazld Are we good to close this issue? You should expect to see more samples/docs around response generation in the future. I'm assuming your 500 is now a 400 with the changes on master?

dazld commented 7 years ago

go ahead! all seems ok for now. will open a more specific issue if appropriate later.

ohpauleez commented 7 years ago

Thanks!