Repository navigation
fix(support): no refund on a timed-out confirmation, UTC-day-safe budget refund, stale-IP eviction (T-6911) - #668
Merged
Conversation
… refunds to their UTC day, sweep stale IPs Leftovers from the re-review of #645 (T-6911). - A confirmation whose 5 s abort fires after the request was sent used to be reported as failed and refunded the address slot and the IP/daily budget, so a retry could mail the submitter twice. postToSendly now resolves "unknown" when our own abort fired; the route keeps every allowance for it and the response reports confirmation: "unknown" (a new neutral ConfirmationLegStatus). The team-notification leg still maps a timeout to "failed" (no refund to protect there). - releaseConfirmationBudget decremented today.count even after the UTC day rolled over. It now refunds the daily cap only when the IP newest confirmation was taken on the current UTC day, and does nothing for an IP with no record. The hourly IP allowance is still returned. - byIp kept expired entries until the 10k cap. Entries whose hour has passed are now swept at most once a minute (and before the hard cap evicts a live IP); the hard cap stays. RED (tests only, before the fix): FAIL confirmation-throttle.test.ts > budget release across the UTC day boundary > does not refund the new day when the slot was taken the day before FAIL confirmation-throttle.test.ts > budget release across the UTC day boundary > does not count down an IP it has no record of FAIL confirmation-throttle.test.ts > stale per-IP entries > drops IPs whose hour has passed once traffic arrives, keeping the map small FAIL confirmation-throttle.test.ts > stale per-IP entries > keeps an IP that is still inside its hour when expired ones are swept FAIL support-route.test.ts > ... > aborts the confirmation send after 5 seconds instead of holding the response FAIL support-route.test.ts > ... > keeps the address allowance and IP budget, and reports unknown, when the 5 s abort fires after the request went out Test Files 2 failed (2) | Tests 6 failed | 67 passed (73) GREEN (after the fix): Test Files 2 passed (2) | Tests 73 passed (73); full landing suite 58 files / 813 tests passed; landing typecheck clean. The existing 5 s abort test now expects "unknown" instead of "failed".
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
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.
Fixes the LOW leftovers from the #645 re-review (T-6911). Items 3 and 4 are deliberate policy and stay unchanged: the 200/day global cap can be used up by about 5 IPs, and requests with no IP share one
unknownbucket, which fails closed.Changes
postToSendlynow resolvesunknown, checked viacontroller.signal.aborted.confirmation: "unknown", a new neutralConfirmationLegStatus.failed.releaseConfirmationBudgetdecrements the daily cap only when the IP's newest confirmation was taken on the current UTC day. The hourly IP allowance is still returned, and an IP with no record refunds nothing.byIpentries are evicted. Entries whose hour has passed are swept at most once a minute, and before the hard cap would evict a live IP. The 10k hard cap stays.Behavior notes
unknowninstead offailed. That is the intended change.confirmation, so nothing user-facing changes.@useupup/landingis in theignorelist of.changeset/config.json.Test plan (run on devino-forth)
Test Files 2 failed (2) | Tests 6 failed | 67 passed (73)Test Files 2 passed (2) | Tests 73 passed (73)Known tradeoff (from review)
fetchcannot tell an abort while connecting from an abort after the body was sent. So if Sendly hangs at connect, the confirmation is also reported asunknown, and the slot and budget are kept, not refunded. This direction over-counts during a Sendly outage and can never mail a submitter twice. An outage then shows up asthrottledon retry instead of as duplicate mail.