Wait out a concurrent token refresh instead of failing the caller - #313
Open
gynsus wants to merge 1 commit into
Open
Wait out a concurrent token refresh instead of failing the caller#313gynsus wants to merge 1 commit into
gynsus wants to merge 1 commit into
Conversation
…e row When RefreshSocialToken and VerifyUpcomingPostConnections fire on the same scheduler tick, the verifier's refreshToken() lost the refresh lock race, immediately re-read the account — before the winning process had saved the new token — and retried verification with the old token, which had just crossed its expiry. The account was then wrongly marked token_expired and disconnected mid-schedule. Block on the lock (up to 15s) so the winner's freshly saved token is what gets re-read.
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.
The race
When a token sits right at its expiry, the verify cron and a scheduled publish fire on the same tick and both call
refreshToken(). The loser of the lock currently re-reads the row immediately — but the winner hasn't persisted the new token yet, so the loser re-reads the old token and fails its caller with "a refresh is already in progress", even though a perfectly fresh token lands milliseconds later.We hit this in production as a YouTube account flapping to "session expired" once a day with nothing actually wrong: both jobs collided on the same cron tick, the loser verified with the stale token, got a 401, and marked the account disconnected. (On current
mainthe failure mode is softer — a transientPlatformUnavailableException— but it still fails a publish attempt that had no reason to fail.)The fix
On a lock miss, wait for the concurrent refresh to finish before re-reading:
15 seconds comfortably covers a provider round-trip; on timeout the behavior degrades to exactly what it is today. The existing
is_token_expiredguard after the re-read stays as the backstop.Testing
ConnectionVerifierTestpasses unchanged — the fix only changes when the row is re-read, not any observable contract. The race itself was verified live: after deploying this, the daily false "session expired" on the colliding account stopped.