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

Return "Bad Request" for errors caused by client #827

Merged
merged 1 commit into from Sep 20, 2014

Conversation

Projects
None yet
2 participants
@schuetzm
Contributor

schuetzm commented Sep 13, 2014

Currently, enforce() is used to check for invalid requests to the server, for example malformed HTTP queries, or missing/wrongly typed query parameters in APIs. enforce() throws a generic Exception, which makes the server return 500 Internal Server Error to the client. However, the appropriate response for invalid requests is 400 Bad Request.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Sep 18, 2014

Member

Thanks, makes sense. The only thing that is a bit ugly is the two overloads of enforceHTTP that only differ in the order of their arguments. Maybe instead of making the status parameter a default one, it would be better to introduce a separate enforceHTTPRequest(cond, message) (any better name?) function?

Member

s-ludwig commented Sep 18, 2014

Thanks, makes sense. The only thing that is a bit ugly is the two overloads of enforceHTTP that only differ in the order of their arguments. Maybe instead of making the status parameter a default one, it would be better to introduce a separate enforceHTTPRequest(cond, message) (any better name?) function?

@schuetzm

This comment has been minimized.

Show comment
Hide comment
@schuetzm

schuetzm Sep 19, 2014

Contributor

I renamed it to enforceBadRequest.

Contributor

schuetzm commented Sep 19, 2014

I renamed it to enforceBadRequest.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Sep 20, 2014

Member

Thanks! Looks like the commit is based on an old failing commit. Everything looks fine though, so I'll just merge like it is.

Member

s-ludwig commented Sep 20, 2014

Thanks! Looks like the commit is based on an old failing commit. Everything looks fine though, so I'll just merge like it is.

s-ludwig added a commit that referenced this pull request Sep 20, 2014

Merge pull request #827 from schuetzm/enforce-bad-request
Return "Bad Request" for errors caused by client

@s-ludwig s-ludwig merged commit 37b3cf0 into vibe-d:master Sep 20, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build could not complete due to an error
Details

@s-ludwig s-ludwig removed the needs input label Sep 20, 2014

s-ludwig added a commit that referenced this pull request Sep 20, 2014

@schuetzm schuetzm deleted the schuetzm:enforce-bad-request branch Sep 21, 2014

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