refactor(ads/admob): propagate FailureReason through poller and dispatcher#6842
Merged
Conversation
…tcher Internal-only change to carry a structured failure reason out of the verification polling loop. No behavior change to the public RewardVerificationResult.failed, which stays parameter-less — mapOutcome in RewardedAds+RewardVerification continues to discard the reason. The new payload becomes useful when an upcoming change exposes failure reasons on a reward-failed-to-verify event. Outcome: - .failed gains an associated FailureReason payload (.timeout / .backendError / .unknown). Poller.run classification: - Loop budget exhausted (all attempts pending, transient throws, or unknown statuses): .failed(.timeout). - Backend returns an explicit .failed status: .failed(.backendError). - Non-transient ErrorCode throw: .failed(.backendError). - Any other throw (CancellationError, unrecognised error types): .failed(.unknown). - Task.isCancelled before an attempt: .failed(.unknown). Dispatcher separately swallows the delivery on real cancellation, so this is only observable in tests. Tests tightened: Outcome / Poller / Dispatcher tests now assert the specific FailureReason for each classification path. Stale comment on testCancellationErrorFromPollStatusReturnsFailedWithoutRetrying (claiming Poller didn't distinguish CancellationError from non-transient throws) updated.
ajpallares
reviewed
May 26, 2026
ajpallares
left a comment
Member
There was a problem hiding this comment.
The changes make sense, but I don't see the new FailureReason being read anywhere (other than tests). How is this going to be used? I wonder mostly in case we need to expose something else or differently
ajpallares
approved these changes
May 26, 2026
Drop redundant Outcome equality test (per review feedback) and pass a FailureReason when constructing .failed in PresentMappingTests.
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
3 participants
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.
Checklist
FailureReasonper classification path)Motivation
The verification polling loop currently collapses every non-verified terminal state into a single
Outcome.failedcase. As a result, downstream consumers can't tell why verification failed — timeout, backend rejection, network noise, or something unrecognised. Carrying that information out ofPolleropens the door for surfacing a structured failure reason on future telemetry / events without changing the publicRewardVerificationResult.failed.Description
RewardVerification.Outcome.failedgains an associatedFailureReasonpayload (.timeout/.backendError/.unknown).Poller.runclassifies failures:.failed(.timeout)..failedstatus →.failed(.backendError).ErrorCodethrow →.failed(.backendError).CancellationError, unrecognised error types) →.failed(.unknown).Task.isCancelledbefore an attempt →.failed(.unknown).Dispatcherseparately swallows the delivery on real cancellation, so this is only observable in unit tests.Dispatcher'slogDescriptionincludes the reason for debugging.mapOutcomeinRewardedAds+RewardVerificationcontinues to pattern-match.failedwithout binding, so the publicRewardVerificationResult.failedstays parameter-less.Test plan
swift build(adapter + tests) +swiftlintclean.OutcomeTests: case construction asserts the new associated value; exhaustive-switch test enumerates all threeFailureReasonvariants.PollerTests: every previously barecase .failed = outcomenow asserts the specific reason produced by that classification path.DispatcherTests: same tightening.CancellationErroris not anErrorCode, so it falls through to the catch-all and surfaces.unknown).Note
Low Risk
Internal-only typing and classification in the AdMob adapter; public reward verification results are unchanged.
Overview
Internal AdMob reward verification now attaches a
FailureReason(.timeout,.backendError,.unknown) toOutcome.failedinstead of a bare failure case, so the polling loop can distinguish why SSV did not succeed.Poller.runmaps outcomes: exhausted attempt budget →.timeout; explicit backend.failedstatus or non-transientErrorCode→.backendError; cancellation before polling,CancellationError, and other unclassified throws →.unknown.Dispatcherlogging includes the reason inlogDescription.mapOutcomestill collapses any.failedto the public parameter-lessRewardVerificationResult.failed, so there is no app-facing API change—only richer internal signal for future telemetry. Unit tests assert the specific reason on each path.Reviewed by Cursor Bugbot for commit 84d05d0. Bugbot is set up for automated code reviews on this repo. Configure here.