diem / dip

Libra Improvement Proposals
https://lip.libra.org
Apache License 2.0
40 stars 55 forks source link

LIP-1 - Error handling #78

Open udirom opened 3 years ago

udirom commented 3 years ago
  1. "Note: Additional information detailing all errors will be added." - I saw protocol & command errors do exist in reference implementation. Maybe they should be added to the document as well?
  2. It is not clear from the description of how error handling should be handled at the HTTP level. Are protocol errors should be returned with 200OK?
longbowlu commented 3 years ago

@kphfb

kphfb commented 3 years ago

@gdanezis I know awhile back that you were putting together a listing of the error codes. Should we either put out a new LIP for error handling or point this LIP to the code?

gdanezis commented 3 years ago

Hi all,

On 1, the reference api has a list of errors it emits here: https://github.com/libra/off-chain-reference/blob/master/src/offchainapi/errors.py Happy to add more docs or add an appendix to the LIP1 to enumerate and document these. Also open to feedback about what other error codes could be useful.

On 2, we currently should be sending 200 OK if the command was successful and 400 if it is a failure: https://github.com/libra/off-chain-reference/blob/eba72f53d4962fccd2033c98500c06bfaaacf4e2/src/offchainapi/asyncnet.py#L174

G

yanivmo commented 3 years ago

On 1, the reference api has a list of errors it emits here: https://github.com/libra/off-chain-reference/blob/master/src/offchainapi/errors.py Happy to add more docs or add an appendix to the LIP1 to enumerate and document these. Also open to feedback about what other error codes could be useful.

On 2, we currently should be sending 200 OK if the command was successful and 400 if it is a failure: https://github.com/libra/off-chain-reference/blob/eba72f53d4962fccd2033c98500c06bfaaacf4e2/src/offchainapi/asyncnet.py#L174

This could be a temporary solution, for now, but eventually it should be defined in a LIP.