Payment failures are no longer populated as structured errors, payment failure details not returned #5477
Milestone
Comments
Roasbeef
added a commit
to Roasbeef/lnd
that referenced
this issue
Jul 7, 2021
In this commit, we fix a regression introduced by a recent bug fix in this area. Before this change, we'd inspect the error returned by `processSendError`, and then fail the payment from the PoV of mission control using the returned error. A recent refactoring removed `processSendError` and combined the logic with `tryApplyChannelUpdate` in order to introduce a new `handleSendError` method that consolidates the logic within the `shardHandler`. Along the way, the behavior of the prior check was replicated in the form of a new internal `failPayment` closure. However, the new function closure ends up returning a `channeldb.FailureReason` instance, which is actually an `error`. In the wild, when `SendToRoute` fails due to an error at the destination, then this new logic caused the `handleSendErorr` method to fail with an error, returning an unstructured error back to the caller, instead of the usual payment failure details. We fix this by no longer checking the `handleSendErorr` for an error as normal. The `handleSendErorr` function as is will always return an error of type `*channeldb.FailureReason`, therefore we don't need to treat it as a normal error. Instead, we check for the type of error returned, and update the control tower state accordingly. With this commit, the test added in the prior commit now passes. Fixes lightningnetwork#5477.
matheusd
pushed a commit
to matheusd/dcrlnd
that referenced
this issue
Nov 14, 2023
In this commit, we fix a regression introduced by a recent bug fix in this area. Before this change, we'd inspect the error returned by `processSendError`, and then fail the payment from the PoV of mission control using the returned error. A recent refactoring removed `processSendError` and combined the logic with `tryApplyChannelUpdate` in order to introduce a new `handleSendError` method that consolidates the logic within the `shardHandler`. Along the way, the behavior of the prior check was replicated in the form of a new internal `failPayment` closure. However, the new function closure ends up returning a `channeldb.FailureReason` instance, which is actually an `error`. In the wild, when `SendToRoute` fails due to an error at the destination, then this new logic caused the `handleSendErorr` method to fail with an error, returning an unstructured error back to the caller, instead of the usual payment failure details. We fix this by no longer checking the `handleSendErorr` for an error as normal. The `handleSendErorr` function as is will always return an error of type `*channeldb.FailureReason`, therefore we don't need to treat it as a normal error. Instead, we check for the type of error returned, and update the control tower state accordingly. With this commit, the test added in the prior commit now passes. Fixes lightningnetwork#5477.
matheusd
pushed a commit
to decred/dcrlnd
that referenced
this issue
Nov 20, 2023
In this commit, we fix a regression introduced by a recent bug fix in this area. Before this change, we'd inspect the error returned by `processSendError`, and then fail the payment from the PoV of mission control using the returned error. A recent refactoring removed `processSendError` and combined the logic with `tryApplyChannelUpdate` in order to introduce a new `handleSendError` method that consolidates the logic within the `shardHandler`. Along the way, the behavior of the prior check was replicated in the form of a new internal `failPayment` closure. However, the new function closure ends up returning a `channeldb.FailureReason` instance, which is actually an `error`. In the wild, when `SendToRoute` fails due to an error at the destination, then this new logic caused the `handleSendErorr` method to fail with an error, returning an unstructured error back to the caller, instead of the usual payment failure details. We fix this by no longer checking the `handleSendErorr` for an error as normal. The `handleSendErorr` function as is will always return an error of type `*channeldb.FailureReason`, therefore we don't need to treat it as a normal error. Instead, we check for the type of error returned, and update the control tower state accordingly. With this commit, the test added in the prior commit now passes. Fixes lightningnetwork#5477.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Background
When making a payment using a send to a route directly, a payment failure normally would return a structured failure message with the failure code
This return value has changed to no longer return a structured failure, it now populates an unstructured error that is missing failure details.
Ideally the behavior for payment failures such as paying to an unknown payment hash remains consistent, and the full set of failure data is returned to the sender
The removal of structured failure data seems to have happened in #5332 with commit
cf2b5744a155c02b34ccb0e1d062f5e70e6fa7b4Your environment
Steps to reproduce
Expected behavior
Actual behavior
The text was updated successfully, but these errors were encountered: