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

pay: Pre-flight checks and better error messages #7418

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Jun 20, 2024

We have long had an issue with the "Ran out of routes" error
essentially covering up the real reason a payment has failed. Let's
try and add some better error messages as well as do some additional
checks that can tell us what went wrong in the first place.

@cdecker cdecker force-pushed the 20240619-preflight-cap-check branch 2 times, most recently from 4e4610d to bc7e939 Compare August 7, 2024 14:45
@cdecker cdecker marked this pull request as ready for review August 7, 2024 14:45
@cdecker cdecker added this to the v24.08 milestone Aug 7, 2024
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack 834997c

Nits: needs folding in the fixup and removing "connected" parsing....

err = json_scan(tmpctx, buf, channel,
"{spendable_msat?:%,peer_connected:%}",
JSON_SCAN(json_to_msat, &spendable),
JSON_SCAN(json_to_bool, &connected));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you grab "connected" and not use it?

This allows us to directly returnan error code based on where we
decided to abort, rather than attemtping to infer it from the parts.

Changelog-Added: pay: The pay plugin now returns better error codes
Changelog-Added: pay: The pay plugin now checks whether we have enough spendable capacity before computing a route, returning a clear error message if we don't
@cdecker cdecker force-pushed the 20240619-preflight-cap-check branch from 834997c to c1eee7c Compare August 8, 2024 08:49
Should make debugging based on logs a bit simpler.
Changelog-Changed: pay: Improved logging should facilitate debugging considerably.
@cdecker cdecker force-pushed the 20240619-preflight-cap-check branch from c1eee7c to 2d50a53 Compare August 8, 2024 08:50
@ShahanaFarooqui ShahanaFarooqui merged commit 89f01f1 into ElementsProject:master Aug 8, 2024
37 checks passed
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

Successfully merging this pull request may close these issues.

3 participants