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

No additional error message in case of 4xx responses #260

Closed
2 tasks done
piotr-iohk opened this issue May 14, 2019 · 1 comment
Closed
2 tasks done

No additional error message in case of 4xx responses #260

piotr-iohk opened this issue May 14, 2019 · 1 comment
Assignees

Comments

@piotr-iohk
Copy link
Contributor

piotr-iohk commented May 14, 2019

Release Operating System Cause
next Linux Code

Context

For some 4xx response codes returned by the API we do deliver additional error message indicating what is wrong with the request (These are mainly true for 400 responses which are most often due to invalid request parameters - additional message often explains what is wrong with particular message, like passphrase is too long...). Some of the 4xx responses however are missing those additional messages.

No additional message is delivered in the following cases:

  • 409 creating wallet that already exists: POST v2/wallets -> no additional message, could be "Wallet already exists"
  • 404 in case of accessing non-existing wallet id GET, PUT, DELETE v2/wallets/{wallet-id} -> no additional message, could be "Wallet with given id not found"
  • 403 in case of providing incorrect old_passphrase (e.g. PUT v2/wallets/{wallet-id}/passphrase) -> no message, could be "Provided old_passphrase is incorrect"
  • 405 (e.g. DELETE v2/wallets) -> could be "HTTP Method not allowed" (if there's option to fine tune it could be "DELETE method not allowed for endpoint v2/wallets)
  • 406 (e.g. http header Accept: text/plain) -> could be "Content not acceptable. Acceptable content is application/json. Check your request's Accept header."
  • 415 (e.g. no Content-Type header for POST and PUT requests) -> could be "Unsupported media type, check Content-Type and Content-Encoding headers of your request"

Although 405, 406, 415 messages would not be so common and may be considered as nice to have I think that having messages for 403, 404 and 409 would be improvement in user experience.

Steps to Reproduce

Look at the cases in the Context above.

Expected behavior

Additional error message could be presented in case of the mentioned 4xx responses.

Actual behavior

There is no additional error message.


Resolution Plan

  • Add proper error description to errors
  • Review error return type to be a JSON object containing both a { "code": ..., "message": ... }

PR

Number Base
#298 master
#299 KtorZ/260/more-descriptive-errors
#308 master
#304 master

QA

  • We now return more descriptive error messages for all errors we do catch and handle in the wallet layer.

  • Also, with a small middleware sitting between Servant and Warp, we now catch "raw" errors thrown directly by servant and provide a helpful payload to them to make them more descriptives too.

  • See also: Cardano.Wallet.Api.Server

@piotr-iohk
Copy link
Contributor Author

well done :) 👍

@KtorZ KtorZ added this to the Bugs Sprint 21-22 milestone May 31, 2019
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