lnd icon indicating copy to clipboard operation
lnd copied to clipboard

track and send payment API endpoints should send consistent responses indicating payment state.

Open razorman8669 opened this issue 1 year ago • 5 comments

Background

When interacting with the API endpoints for /v2/router/track and /v2/router/send, it's particularly difficult to determine if a payment actually failed, or if some other type of error occured, due to the inconsistent response structures returned for various error states.

For instance, when trying to create multiple failure states, I have received many different response formats from these endpoints, such as:

  • {"code":3,"message":"unexpected EOF","details": []}
  • {"code":3,"message":"type mismatch, parameter: payment_hash, error: illegal base64 data at input byte 0","details":[]}
  • {"error":{"code":2,"message":"invoice expired. Valid until 2022-07-05 09:07:39 +0000 UTC","details":[]}}
  • {"error":{"code":6,"message":"invoice is already paid","details":[]}}
  • {"error":{"code":5,"message":"payment isn't initiated","details":[]}}
  • {"result": {..., "status":"FAILED","failure_reason":"FAILURE_REASON_ERROR",...}}

As you can see, there are 3 different formats that I have discovered thus far, and theres most likely many others that I haven't yet seen. This makes it difficult to determine, with 100% certainty, that a payment did indeed FAILED or if something else is going on (connection/networking issues, lnd hiccups, malformed request params, etc). as it is, clients that consume these responses need to have handlers for every type of possible "error message" to determine if it's actually "FAILED" (like "payment isn't initiated") or if there's some networking issue where the request itself failed, but the payment DID NOT (like "unexpected EOF" or "type mismatch, parameter: payment_hash, error: illegal base64 data at input byte 0")

Your environment

  • version of lnd
    • v0.15.0-beta
  • which operating system (uname -a on *Nix)
    • Ubuntu 18.04.6 LTS
  • version of btcd, bitcoind, or other backend
    • btcd version 0.22.0-beta

Steps to reproduce

send bad requests to the /v2/router/track and /v2/router/send endpoints.

Expected behaviour

I would expect the payment responses to tell me, with certainty, that either the payment is "FAILED", "SUCCEDED", or "INFLIGHT".

Actual behaviour

I get error messages wrapped in error properties, or in top level objects, or in the result property, and am required to know every possible error state that could happen in order to handle payments responses properly.

razorman8669 avatar Jul 18 '22 20:07 razorman8669

Just a quick side note here that these are both streaming APIs and you should use WebSockets for them. That likely explains the EOF error. The illegal base64 data at input byte 0 error is because you need to encode all bytes fields as base64 in REST.

guggero avatar Jul 19 '22 14:07 guggero

Thanks @guggero, I do understand the cause of those error, but the point is that I had determined that there was an error format that was different from the others. and I'm assuming there could be others like those which might need to be handled properly, but that's just the ones I know of now. The main point of this issue is to note that it's difficult to determine a true "failed" payment vs an "error". even for the track endpoint, we still get error responses without a Failed/Succeded status and it's unknown if those errors are due to sending the wrong format/protocol/metadata or if it's actually failed

razorman8669 avatar Jul 19 '22 20:07 razorman8669

for instance, payment isn't initiated error returned by the track endpoint is actually a FAILED payment. but it gets returned as 'error'. it will never resolve as SUCEEDED because it was never actually sent.

razorman8669 avatar Jul 19 '22 20:07 razorman8669

I guess it's a bit weird indeed if you just look at the REST API. But because REST is just a wrapper for gRPC, it inherits its patterns. Basically every RPC has two response types: A generic error (that's when the error object is set in the response) and the intended RPC response message of the documented type (that's when the result object is set in the response). So a general rule of thumb is: If it's a generic error (e.g. if (response.error) {), then nothing was initiated. If it's a typed response (if (response.result) {), it's an application specific response and you need to parse the message according to the API docs and then interpret the response (which might still contain a failure case, but then usually accompanied with more details).

I'm not sure how we can document that better. I guess even if you're using REST, it might be worth taking a look at the actual gRPC interface that defines all the calls (see the *.proto and *.yaml files in the lnrpc directory and its sub-directories).

guggero avatar Jul 20 '22 16:07 guggero

A tricky aspect here is that if you want to see the state of a past payment that did not succeed, it can return an error response because the payment was deleted or never "really" started, or sometimes it can return a success that shows that it failed, like via no route etc. It might be more consistent for callers if the "payment not found" return value would be moved from an error to a response value and defined within the enum that shows possible payment states.

alexbosworth avatar Jul 26 '22 18:07 alexbosworth