Clarify hold invoice cancellation errors#4085
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the error message for incorrect payment details in the English locale to be more descriptive and adds a mapping for raw LND error strings to ensure they are translated correctly. It also includes updated unit tests for these changes. A review comment suggests updating the logic that appends Keysend-specific hints to use the locale key instead of the enum string, ensuring that raw LND errors also receive the appropriate context-specific messaging.
9fdb2cb to
09802e1
Compare
|
Hi @newmattock, please make sure to include the full PR template when opening pull requests. You can find more details in our contribution guidelines |
|
Per the Contribution Guidelines, first time contributors are required to include screenshots or screen recordings of their changes. |
|
Thanks, acknowledged. I do not currently have real device or simulator media for this LND error mapping path, so I will not attach synthetic evidence. The PR body lists the unit/static validation, including the raw LND REST error test; I will add real UI evidence if/when I can reproduce the canceled hold-invoice path on a device or simulator. |
Description
Relates to issue: #2001
This PR makes the LND "Payment details incorrect" response clearer for users, especially when a hold invoice has been canceled. It maps the raw LND REST error text to the existing incorrect-payment-details locale key, updates the English copy to explain that the invoice may have been rejected, canceled, or expired, and keeps the Keysend-specific hint appended through the locale-key path.
This pull request is categorized as a:
Checklist
yarn run tscand made sure my code compiles correctlyyarn run lintand made sure my code didn't contain any problematic patternsyarn run prettierand made sure my code is formatted correctlyyarn run testand made sure all of the tests passValidation run for this focused patch:
./node_modules/.bin/prettier --check utils/ErrorUtils.ts utils/ErrorUtils.test.ts locales/en.json./node_modules/.bin/eslint utils/ErrorUtils.ts utils/ErrorUtils.test.ts./node_modules/.bin/jest utils/ErrorUtils.test.ts --runInBand./node_modules/.bin/tsc --noEmit --pretty falseCurrent GitHub PR checks are passing on the PR:
Maintainer-requested first-time-contributor media is still not attached. This patch changes utility error mapping and locale text, with behavior covered by
utils/ErrorUtils.test.ts; no synthetic screenshot or recording has been attached.Testing
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
Not device-tested. This is a utility/error-message mapping change covered by
utils/ErrorUtils.test.ts.I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
On-device
Remote
The new test covers the raw LND REST text:
Payment details incorrect (unknown hash, invalid amt or invalid final cltv delta).Locales
Third Party Dependencies and Packages
yarnafter this PR is merged inpackage.jsonandyarn.lockhave been properly updatedOther:
Fixes #2001