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

pattern for returning status codes: 400 vs 404 vs 422 #23

Open
eoln opened this issue Jul 28, 2020 · 3 comments
Open

pattern for returning status codes: 400 vs 404 vs 422 #23

eoln opened this issue Jul 28, 2020 · 3 comments
Assignees

Comments

@eoln
Copy link
Contributor

eoln commented Jul 28, 2020

@MichaelJBRichards @jgeewax @lewisdaly @bushjames @elnyry-sam-k

We need to agree on the pattern of how we should return HTTP status codes when doing validation of complex requests. For example in POST /thirdpartyRequests/transactions/{ID}/authorizations we have request payload body defined as

export interface AuthPayload {
  consentId: string;
  sourceAccountId: string;
  status: 'PENDING' | 'VERIFIED';
  challenge: string;
  value: string;
}

the code: https://github.com/mojaloop/auth-service/pull/22/files#diff-7da2816b4c61411199493eb4e1667117

we already have payload validated according open-api specification and if needed 400 (Bad Request) status code is return
and we are doing some domain-specific semantic validations, like

  • if Consent for givenconsentId has the proper status
  • if there are matching Scopes for Consent

here the question arises: should we return 404 (Not Found) if there is nothing in our store or 400 (Bad Request),
or maybe 422 (Unprocessable entity) because we have valid request body - open-api schema validation is successful, so Bad Request is not applicable because there is no syntax error.
404 is also not applicable - it should be used when the server has not found anything matching the Request-URI - and the URI is valid.

PROPOSED Pattern:
400 (BadReqeust) only for request syntax errors
404 (Not Found) when the Request-URI is invalid and also if any resource referenced by URL parameter can't be found in store
422 (Unprocessable entity) when semantic validation fails: any resource referenced in payload body can't be found or can't be used to fulfill the request (not enough data properties, not expected internal state not found related entities and so on)

I am assigning the issue to @MichaelJBRichards because he is a member of the API control board and I believe this decision should get approval from the board.

@eoln
Copy link
Contributor Author

eoln commented Jul 29, 2020

let finish handler ASAP (after doing cheap validation which could result in 4**) with sending status 202/201
and schedule background job to do expensive semantic validation which needs to make a lookup to another resource (MySQL) and if any semantic validation error happens let us invoke PUT /..../error resource to inform the client about it.

Keeping open incoming connections until we reach MySQL to do expensive validation could result in slow response and drain totally out connections pool when high traffic arrives.

Does it fit you @jgeewax ???

@jgeewax
Copy link
Contributor

jgeewax commented Jul 30, 2020

@eoln : Sounds great to me :-) Appreciate the push to get this merged -- thanks !!

@lewisdaly
Copy link
Contributor

let finish handler ASAP (after doing cheap validation which could result in 4**) with sending status 202/201
and schedule background job to do expensive semantic validation which needs to make a lookup to another resource (MySQL) and if any semantic validation error happens let us invoke PUT /..../error resource to inform the client about it.

Yep, this sounds great to me @eoln - thanks

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

4 participants