Skip to content

Commit 24a4664

Browse files
committed
fix(credentials): keep throttling and outage backoff streaks apart and redact the fallback log in selector mode
1 parent 7b7eb35 commit 24a4664

5 files changed

Lines changed: 99 additions & 24 deletions

File tree

‎apps/sim/lib/credentials/coordinated-client-token.integration.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,44 @@ describe('coordinated token resolution while the provider fails', () => {
414414
).toBeGreaterThanOrEqual(60_000)
415415
})
416416

417+
it('backs off a throttled answer after an outage from a minute, not by the outage streak', async () => {
418+
const exchange = vi.fn<CoordinatedClientTokenInput['exchange']>()
419+
for (const status of [502, 502]) {
420+
exchange.mockImplementation(failing(status))
421+
await expect(
422+
resolveCoordinatedClientToken(input('outage-then-throttled', exchange))
423+
).rejects.toThrow()
424+
await expireFailureRowsOf('outage-then-throttled')
425+
}
426+
exchange.mockImplementation(failing(429))
427+
const before = Date.now()
428+
429+
await expect(
430+
resolveCoordinatedClientToken(input('outage-then-throttled', exchange))
431+
).rejects.toMatchObject({ status: 429 })
432+
433+
const replayMs = (await failureRowOf('outage-then-throttled')).expiresAt.getTime() - before
434+
expect(replayMs).toBeGreaterThanOrEqual(60_000)
435+
expect(replayMs).toBeLessThanOrEqual(72_000 + 2_500)
436+
})
437+
438+
it('backs off throttling that continues for every worker longer than its first minute', async () => {
439+
const exchange = failing(429)
440+
await expect(
441+
resolveCoordinatedClientToken(input('throttled-twice', exchange))
442+
).rejects.toThrow()
443+
await expireFailureRowsOf('throttled-twice')
444+
const before = Date.now()
445+
446+
await expect(
447+
resolveCoordinatedClientToken(input('throttled-twice', exchange))
448+
).rejects.toThrow()
449+
450+
expect(
451+
(await failureRowOf('throttled-twice')).expiresAt.getTime() - before
452+
).toBeGreaterThanOrEqual(96_000)
453+
})
454+
417455
it.each([
418456
[
419457
'the still-valid token while its refresh is remembered failing',

‎apps/sim/lib/credentials/coordinated-client-token.ts‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
requestTokenWithRetry,
1212
TOKEN_EXPIRY_MARGIN_MS,
1313
TOKEN_FAILURE_STREAK_RETENTION_MS,
14+
type TokenRequestFailure,
1415
} from '@/lib/credentials/token-request-resilience'
1516
import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'
1617
import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
@@ -92,13 +93,15 @@ interface StoredApplicationToken {
9293
/**
9394
* A remembered exchange failure, replayed until its row's `expiresAt`. Builds before failure
9495
* kinds stored `{ code, status }` only and ignore other keys, so `consecutiveFailures` is
95-
* written only for outages and throttling, to grow their backoff while they continue, and
96-
* `refusal` only for a refusal whose code those builds read as `provider_unavailable`.
96+
* written only for outages and throttling, to grow their backoff while they continue, `throttled`
97+
* only for throttling, so its streak is kept apart from an outage's, and `refusal` only for a
98+
* refusal whose code those builds read as `provider_unavailable`.
9799
*/
98100
interface StoredFailure {
99101
code: 'invalid_credentials' | 'grant_not_permitted' | 'site_not_found' | 'provider_unavailable'
100102
status: number
101103
consecutiveFailures?: number
104+
throttled?: true
102105
refusal?: true
103106
}
104107

@@ -206,6 +209,7 @@ function decodeFailure(value: string, step: string): StoredFailure {
206209
...(Number.isSafeInteger(parsed.consecutiveFailures) && Number(parsed.consecutiveFailures) > 0
207210
? { consecutiveFailures: Number(parsed.consecutiveFailures) }
208211
: {}),
212+
...(parsed.throttled === true ? { throttled: true } : {}),
209213
...(parsed.refusal === true ? { refusal: true } : {}),
210214
}
211215
}
@@ -230,20 +234,24 @@ function replayedFailure(
230234
}
231235

232236
/**
233-
* Outages and throttled answers in a row before this exchange, read from the application's last
234-
* failure memo for this secret once its replay has ended. Anything unreadable starts a new streak.
237+
* The outage or throttling streak before this exchange, read from the application's last failure
238+
* memo for this secret once its replay has ended. A refusal, or anything unreadable, starts a new
239+
* streak.
235240
*/
236-
async function consecutiveFailuresBefore(
241+
async function failureStreakBefore(
237242
row: StoredRow | undefined,
238243
step: string
239-
): Promise<number> {
240-
if (!row || Date.now() - row.expiresAt.getTime() > TOKEN_FAILURE_STREAK_RETENTION_MS) return 0
244+
): Promise<Pick<TokenRequestFailure, 'kind' | 'consecutiveFailures'> | undefined> {
245+
if (!row || Date.now() - row.expiresAt.getTime() > TOKEN_FAILURE_STREAK_RETENTION_MS) return
241246
try {
242247
const failure = decodeFailure((await decryptSecret(row.encryptedValue)).decrypted, step)
243-
if (isRememberedRefusal(failure)) return 0
244-
return failure.consecutiveFailures ?? 1
248+
if (isRememberedRefusal(failure)) return
249+
return {
250+
kind: failure.throttled ? 'throttled' : 'transient',
251+
consecutiveFailures: failure.consecutiveFailures ?? 1,
252+
}
245253
} catch {
246-
return 0
254+
return
247255
}
248256
}
249257

@@ -418,7 +426,7 @@ export async function resolveCoordinatedClientToken(
418426
if (!(error instanceof TokenServiceAccountValidationError)) throw error
419427
const assessed = assessTokenRequestFailure(
420428
error,
421-
await consecutiveFailuresBefore(failureRow, step)
429+
await failureStreakBefore(failureRow, step)
422430
)
423431
const replayMs = Math.max(assessed.replayMs, input.minExchangeIntervalMs ?? 0)
424432
const code = STORED_FAILURE_CODES.has(error.code)
@@ -428,7 +436,10 @@ export async function resolveCoordinatedClientToken(
428436
code,
429437
status: error.status,
430438
...(assessed.kind !== 'permanent'
431-
? { consecutiveFailures: assessed.consecutiveFailures }
439+
? {
440+
consecutiveFailures: assessed.consecutiveFailures,
441+
...(assessed.kind === 'throttled' ? { throttled: true } : {}),
442+
}
432443
: isPermanentTokenFailureCode(code)
433444
? {}
434445
: { refusal: true }),

‎apps/sim/lib/credentials/token-request-resilience.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export type TokenRequestFailureKind = 'permanent' | 'throttled' | 'transient'
1313

1414
export interface TokenRequestFailure {
1515
kind: TokenRequestFailureKind
16-
/** Non-permanent failures in a row, this one included; 1 for a permanent failure. */
16+
/** Failures of this kind in a row, this one included; 1 for a permanent failure. */
1717
consecutiveFailures: number
1818
/** How long the failure is replayed before the provider is asked again. */
1919
replayMs: number
@@ -87,15 +87,17 @@ function boundedBackoff(attempt: number, bounds: { baseMs: number; maxMs: number
8787
* replaying (`replayForMs`) is replayed exactly as long as that layer still does. Otherwise a
8888
* provider's `Retry-After` is honored within 1 s to 5 min; without one, a throttled answer backs
8989
* off from 1 min to 5 min, an outage from 5 s to 30 s, and a refusal is replayed 30 s.
90-
* `previousConsecutiveFailures` counts the non-permanent failures immediately before this one (0
91-
* after a success or a permanent failure).
90+
* `previous` is the remembered failure immediately before this one, if any: the backoff grows only
91+
* while failures of the same kind continue, so a throttled answer after an outage starts its own
92+
* streak at a minute instead of inheriting the outage's count.
9293
*/
9394
export function assessTokenRequestFailure(
9495
error: unknown,
95-
previousConsecutiveFailures = 0
96+
previous?: Pick<TokenRequestFailure, 'kind' | 'consecutiveFailures'>
9697
): TokenRequestFailure {
9798
const kind = classifyTokenRequestFailure(error)
98-
const consecutiveFailures = kind === 'permanent' ? 1 : previousConsecutiveFailures + 1
99+
const consecutiveFailures =
100+
kind !== 'permanent' && previous?.kind === kind ? previous.consecutiveFailures + 1 : 1
99101
const replay = (replayMs: number): TokenRequestFailure => ({
100102
kind,
101103
consecutiveFailures,

‎apps/sim/lib/oauth/credential-service-cancellation.test.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,10 @@ describe('client credential token refresh after a failed mint', () => {
169169
)
170170

171171
/** Resolves with fake timers running, so an in-mint retry pause elapses without real waiting. */
172-
async function resolveFor(credentialId: string, providerId = RAMP_SERVICE_ACCOUNT_PROVIDER_ID) {
172+
async function resolveFor(
173+
credentialId: string,
174+
providerId: string = RAMP_SERVICE_ACCOUNT_PROVIDER_ID
175+
) {
173176
queueTableRows(credential, [storedCredential])
174177
const outcome = resolveServiceAccountToken(credentialId, providerId).then(
175178
(value) => ({ value }),
@@ -356,6 +359,30 @@ describe('client credential token refresh after a failed mint', () => {
356359
})
357360
})
358361

362+
it('waits about a minute, not an outage streak longer, for a throttled answer after an outage', async () => {
363+
const mint = minter()
364+
const answers = [unavailable(), unavailable(), unavailable(429)]
365+
mint.mockImplementation(async () => {
366+
throw answers[0]
367+
})
368+
369+
await resolveFor('ramp-outage-then-throttled')
370+
answers.shift()
371+
vi.advanceTimersByTime(30_001)
372+
await resolveFor('ramp-outage-then-throttled')
373+
answers.shift()
374+
vi.advanceTimersByTime(30_001)
375+
await expect(resolveFor('ramp-outage-then-throttled')).resolves.toEqual({
376+
error: expect.objectContaining({ status: 429 }),
377+
})
378+
mint.mockResolvedValueOnce(token)
379+
380+
vi.advanceTimersByTime(73_000)
381+
await expect(resolveFor('ramp-outage-then-throttled')).resolves.toEqual({
382+
value: { accessToken: token.accessToken },
383+
})
384+
})
385+
359386
it('waits at least 5 seconds before asking again after an outage, whatever the jitter', async () => {
360387
const lowestJitter = vi
361388
.spyOn(globalThis.crypto, 'getRandomValues')

‎apps/sim/lib/oauth/credential-service.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ interface FailedClientCredentialMint {
652652
/** The error the failed mint threw, re-thrown to callers while memoized. */
653653
error: unknown
654654
kind: TokenRequestFailureKind
655-
/** Non-permanent failures in a row; the next one backs off longer. */
655+
/** Failures of this kind in a row; the next one of the same kind backs off longer. */
656656
consecutiveFailures: number
657657
secretFingerprint: string
658658
/** Until when the failure is replayed instead of minting again. */
@@ -819,10 +819,7 @@ async function resolveMintedServiceAccountToken(
819819
return { ...minted.token }
820820
} catch (error) {
821821
if (error instanceof Error && error.name === 'AbortError') throw error
822-
const failure = assessTokenRequestFailure(
823-
error,
824-
failed && failed.kind !== 'permanent' ? failed.consecutiveFailures : 0
825-
)
822+
const failure = assessTokenRequestFailure(error, failed)
826823
const retryAtMs = Date.now() + failure.replayMs
827824
clientCredentialMintFailureCache.set(cacheIdentity, {
828825
error: options?.privacyMode === 'selector' ? new Error('Credential mint failed') : error,
@@ -835,7 +832,7 @@ async function resolveMintedServiceAccountToken(
835832
const fallback = failure.kind === 'permanent' ? undefined : stillValid()
836833
if (fallback) {
837834
logger.warn('Serving a still-valid client-credential token after its refresh failed', {
838-
credentialId,
835+
...(options?.privacyMode === 'selector' ? {} : { credentialId }),
839836
failure: failure.kind,
840837
retryInMs: failure.replayMs,
841838
})

0 commit comments

Comments
 (0)