search

coinbase / mesh-specifications

Specification files for the Mesh Blockchain Standard
Apache License 2.0
327 stars 93 forks source link

Define Error.Code as authoritative #25

Closed matheusd closed 4 years ago

matheusd commented 4 years ago

(Opening up as a PR directly with the suggested change to clarify this point for implementations and clients but happy to discuss if this isn't the actual intention of the Error Code/Message distinction.)

This makes it explicit that the Code attribute of the Error model is the authoritative source of truth for the origin of the error, while the error message is secondary.

It also makes it explicit that the same error code may present multiple different messages so clients of Rosetta implementations should not rely on them for error detection and Rosetta backends should make an effort to present error codes in a granular enough level for clients.

patrick-ogrady commented 4 years ago

Thanks for posting this PR @matheusd (I think it may be the first external!) ... I clarified a point about Error.Message vs Error.Details and think the documentation around these models could definitely be improved to include this.

Would ❤️ if you repurposed this PR to improve the documentation on these Error fields!

matheusd commented 4 years ago

Updated.

Given that the message has to be fixed, should they maybe be recommended to be code-like messages vs descriptive messages?

Stuff like "ErrBlockNotFound" vs "block not found"? Or do you feel this is an unnecessary distinction to make?

patrick-ogrady commented 4 years ago

Thanks for writing these descriptions @matheusd! Some small nits around some extra trailing spaces but once those are fixed looks good.

Given that the message has to be fixed, should they maybe be recommended to be code-like messages vs descriptive messages?

Stuff like "ErrBlockNotFound" vs "block not found"? Or do you feel this is an unnecessary distinction to make?

That's a great question. I don't believe we need to restrict all messages to be code-like (descriptive messages may honestly make things more understandable). Consistency of message and code is the most important point here (which your changes already make clear).

patrick-ogrady commented 4 years ago

(CircleCI didn't get kicked off so I had to close/open the PR)