Use LdkServerError for error handling in all APIs.#46
Use LdkServerError for error handling in all APIs.#46G8XSU merged 24 commits intolightningdevkit:mainfrom
Conversation
G8XSU
force-pushed
the
25-02-error-handling
branch
from
f2e47f5 to
878b52e
Compare
| | NodeError::ChannelCreationFailed | ||
| | NodeError::ChannelClosingFailed | ||
| | NodeError::ChannelConfigUpdateFailed | ||
| | NodeError::DuplicatePayment |
There was a problem hiding this comment.
Haven't looked through these exhaustively, but for BOLT11 DuplicatePayment is returned if the user tries to pay an invoice with the same payment hash twice. So shouldn't it map to an InvalidRequestError? Though for BOLT12 it would happen if we randomly generate the same payment id, which shouldn't happen.
There was a problem hiding this comment.
We can't mark DuplicatePayment as invalid-request with certainty, it could be a legitimate race-condition on client side or idempotency failure.
Ideally, DuplicatePayment should become a sub-error within the lightning error category.
We typically use invalid-request for requests that can be safely ignored or can’t be fixed programmatically, but DuplicatePayment might indicate a successful payment the client didn’t register.
| LdkServerError::new( | ||
| InvalidRequestError, | ||
| "Address is not valid for LdkServer's configured network.".to_string(), | ||
| ) |
There was a problem hiding this comment.
Do we need use the mapping here in order to provide a more specific error?
There was a problem hiding this comment.
Actually, using the mapping makes the error less specific, since we can't customize the error message for specific scenario.
We use it where we can but provide more context by directly using LdkServerError to be more helpful.
There was a problem hiding this comment.
Yeah, I meant to say "not use" not "need use" lol.
G8XSU
force-pushed
the
25-02-error-handling
branch
from
d60055d to
f795976
Compare
|
Squashed fixups. |
2 participants
Use LdkServerError for error handling in all APIs.