Repository navigation
fix(cash-link): report a rejected claim intent as a failure - #1570
Merged
Merged
Conversation
ReceiveGiftCardTransactor.start() mapped the account-creation Result into receiveRemotely(), which returns its own Result. That produced a Result<Result<IntentType>>, and the outer fold only saw the account step. A rejected intent landed in onSuccess and start() returned success. When two people claim the same cash link, the server rejects the second with SubmitIntentError.StaleState (already claimed). Android still called onReceived for that claim: CashLinkDelegate added a phantom local balance, showed a received bill, and emitted SettledClaim(collected = true), which posted a duplicate "thanks for the cash" reply. Analytics logged receiveCashLink as a success. Any other intent failure after the account step took the same path. Fold the account result into receiveRemotely() so the intent's Result is the one folded. The already-claimed stale state is still returned without logAndFail; other failures still go through it. This regressed in 6d502a9.
github-actions
Bot
added
area: network
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
1 participant
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.
ReceiveGiftCardTransactor.start()treated a rejected claim intent as a successful claim. It called.mapon the account-creationResultand ranreceiveRemotely()inside it.receiveRemotely()returns its ownResult, so the value becameResult<Result<IntentType>>. The fold after that only failed when account creation failed, so a failed intent went toonSuccess. This came in with 6d502a9.When two people tap the same cash link, the server accepts one claim and rejects the other with
SubmitIntentError.StaleState(gift card already claimed). On Android the rejected claim still went down the success path:CashLinkDelegateadded a phantom local balance throughtokenCoordinator.addand showed a "received" bill.SettledClaim(collected = true), andChatViewModel.thankForClaimposted a duplicate "thanks for the cash" reply.receiveCashLinkas a success.Any other intent failure after the account step, such as a denial or a network error, took the same path. iOS (
Session.receiveCashLink) already handles the already-claimed case.Changes
receiveRemotely()so the intent'sResultis the one that reachesonFailure. Error handling stays the same: the already-claimed stale state is returned withoutlogAndFail, and every other error goes through it.ReceiveGiftCardTransactorTestcases wherereceiveRemotely()returns an already-claimedStaleStateand aDeniederror. Both fail before the fix.Result.successfromgetAccounts. The new tests do it on MockK 1.14.11.