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

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

Open
zenkimoto opened this issue Mar 10, 2013 · 2 comments
Open

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

zenkimoto opened this issue Mar 10, 2013 · 2 comments

Comments

@zenkimoto
Copy link

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
Copy link
Owner

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
Copy link
Author

@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!

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

No branches or pull requests

2 participants