dannykopping / spore

ReSTful extensions for Slim
29 stars 2 forks source link

Unable to deserialize -> Return HTTP 400 Bad Request? #16

Open zenkimoto opened 11 years ago

zenkimoto commented 11 years ago

Great project, thanks for creating it! I've been using it to create a ReSTful service.

Just wondering what your thoughts are on returning a 400 Bad Request when the Deserializer can't deserialize the request, rather than a 500 error showing exception details. The use case would be in cases when only a partial request (partial JSON for example) made it through to the server.

dannykopping commented 11 years ago

Hey @zenkimoto

Nice idea - I agree with you; this would be a useful feature. I do worry that it'll open up a big can of worms though. Not that it's a problem - I just think it's worth going through the whole project and seeing where certain errors could be thrown and with which codes. Do you have any other suggestions for cases where a different code (other than 500) would be warranted?

Thanks

zenkimoto commented 11 years ago

@dannykopping

Yeah, I don't think opening a big can of worms would be warranted either. I looked through Spore's code and I can't see anywhere else where I'd return anything other than a 500. (Except for the case that I mentioned) Perhaps when I used it more I can make more suggestions later.

After going over this code, I've noticed that the router does not copy the http responses from the result of the middleware chain. I hope you don't mind, but I wanted to help out. :-) I modified the deserializer to return a bad request status. Then in the router, I copied the http status from the Slim response to the Spore response. Then based on the result, determine if the service should be called or not.

I made these changes because I also need to support other custom middleware for Slim. For example, the Slim extra's middleware sets up the 401 status when a user is not authenticated via HTTP. https://github.com/codeguy/Slim-Extras/blob/master/Middleware/HttpBasicAuth.php

Let me know what you think of the changes!