Keep polling the payment wait endpoint when it times out - #1223
Open
TaprootFreak wants to merge 4 commits into
Open
Keep polling the payment wait endpoint when it times out#1223TaprootFreak wants to merge 4 commits into
TaprootFreak wants to merge 4 commits into
Conversation
The endpoint is going to cap how long it blocks and answer 408 while the payment is still pending. fetchJson does not distinguish success from failure, so that answer reaches the success callback, which stops the poll — and init() ignores a repeat call for the same URL, so it never resumes and the payment is never picked up. Treat 408 as "nothing decided yet" and let the poll continue.
Two follow-ups from review: The POS wait retries on error without any delay. Once the endpoint answers 408 that path is taken regularly, and an error returning immediately would turn the retry into a tight loop. It now waits before asking again. usePolling.stop() left url.current set, so init() treated a restart on the same URL as a duplicate and silently did nothing. A wait poll keeps its URL for as long as the payment is pending, which made every stop on a pending payment final. Releasing the URL in stop() removes the trap the 408 guard has to work around, and the added test covers it.
Review follow-up on the retry delay. The retry ran for every error, including the 401 that unauthorizedResponse rethrows after clearing the session. Asking again without a valid key only repeats the same 401, so the terminal kept calling the endpoint every two seconds for as long as it stayed open. The retry now skips that case. The retry timer is also tracked and cleared on unmount, so a pending retry no longer fires for a terminal nobody is using anymore.
Review follow-up on the retry timer. Creating a payment starts a wait of its own, so more than one retry can be pending at a time. A single ref kept only the newest timer id and lost the older one, which then fired after unmount — the case the previous commit meant to rule out. The timers are now held in a set and all of them are cleared on unmount. Adds tests for both retry rules: ask again after a timeout, stop once the session is gone.
Collaborator
Author
|
Four review passes were needed to reach zero findings. Beyond the original one-line guard, the passes surfaced three problems in the paths this change touches:
All are fixed, with tests for the restart and for both retry rules. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The API is about to cap how long
GET /v1/lnurlp/wait/{id}blocks and answer HTTP 408 while the payment is still pending (DFXswiss/api#4502, implemented in DFXswiss/api#4533). Today that request is instead cut off by the CDN after 125 s with a gateway error page.That difference matters here.
fetchJson(src/util/utils.ts) returns the parsed body regardless of status, so today's non-JSON error page makes the promise reject and the poll interval keeps running — the widget recovers by accident. A JSON 408 would instead arrive in the success callback, which callsstopWaitPolling(). AndusePolling.init()returned early when called again with the same URL, which is exactly what happens for a wait poll (the URL is derived from the payment and does not change while it is pending). The poll would stop for good and the completed payment would never be picked up.What
payment-link.context.tsx— a 408 in the wait callback is treated as "nothing decided yet" and the poll continues.polling.ts—stop()now also releasesurl.current. It previously kept it, so a restart on the same URL was silently ignored: every stop on a still-pending payment was final. That was the trap the 408 guard had to work around; it is now gone, and a test covers the restart.payment-link-pos.context.tsx— the POS terminal reaches the same endpoint throughcall(), which throws on any non-2xx, so its.catchchain already re-issued the wait. Three things were wrong with that retry once 408 becomes a regular occurrence:The retry now waits, skips the unauthorized case, and every pending timer is cleared on unmount. Because creating a payment starts a wait of its own, the timers are held in a set rather than a single slot — otherwise the older one is lost and fires unattended.
Tests
polling.test.tscovers the restart after stop;payment-link-pos-wait-retry.test.tsxcovers both retry rules (ask again after a timeout, stop once the session is gone).Deployment order
This has to go out before the API-side cap.