Produce receive::JsonError accurately so that send can properly handle it#506
Merged
spacebear21 merged 4 commits intopayjoin:masterfrom Jan 25, 2025
Merged
Produce receive::JsonError accurately so that send can properly handle it#506spacebear21 merged 4 commits intopayjoin:masterfrom
receive::JsonError accurately so that send can properly handle it#506spacebear21 merged 4 commits intopayjoin:masterfrom
Conversation
Collaborator
Pull Request Test Coverage Report for Build 12966955577Details
💛 - Coveralls |
spacebear21
reviewed
Jan 24, 2025
spacebear21
reviewed
Jan 24, 2025
DanGould
force-pushed
the
json-error
branch
3 times, most recently
from
2a6a042 to
bbc06bb
Compare
Contributor
Author
|
Elided the explicit NOT_ENOUGH_MONEY does not return as much useful information to match feerates as I would like, but it's still on the right track and we can follow up when we classify the v2 errors. I think this is ready to go in. |
Contributor
Author
|
this bare url lint failure wasn't caught when I ran |
Contributor
Author
|
Rebased to fix feerate/fee_rate conflict |
Some errors need to be returned to the sender as JSON. This concern is separate from the `Display` concern and isolated into a `JsonError:to_json` function defined in a trait.
The other codes are unknown and won't be displayed to clients.
Well-known error codes are used in both send and receive modules. Using constants makes it harder for a maintainer to accidentally produce or parse the wrong code.
Perhaps it can be extended to reveal preferences about appropriate feerates with a template, but for now this at least uses the well known error code as intended.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change could also introduce
constvalues for well known error codes for bothsendandreceiveto share to prevent slipped typos during maintenance.In reviewing the error types for v2, I realize we may want to introduce new well-known error types for the
SessionErrorvariants. In particular,Expiredshould have a template string to return to sender clients. One other variants seems to approximately fitoriginal-psbt-rejectedpayload errors (Hpke), but UnexpectedResponseSize, OhttpEncapsulation, and UnexpectedStatusCode appear to be directory issues that are unrecoverable and cannot return json to a sender.Seems like feerate configuration errors are also a special class of recoverable original-psbt-rejected error that might be worth classifying to potentially handle by clever senders.
related: #403