Skip to content

Commit bb2bb1c

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(quickbooks): extend refresh lock budget
1 parent 8badd54 commit bb2bb1c

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

apps/sim/app/api/auth/oauth/utils.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@
77
import { redisConfigMockFns } from '@sim/testing'
88
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
99

10+
const { capturedLeaderLockOptions } = vi.hoisted(() => ({
11+
capturedLeaderLockOptions: [] as Array<Record<string, unknown>>,
12+
}))
13+
vi.mock('@/lib/concurrency/leader-lock', async (importOriginal) => {
14+
const actual = await importOriginal<typeof import('@/lib/concurrency/leader-lock')>()
15+
return {
16+
...actual,
17+
withLeaderLock: vi.fn((options) => {
18+
capturedLeaderLockOptions.push(options as unknown as Record<string, unknown>)
19+
return actual.withLeaderLock(options)
20+
}),
21+
}
22+
})
23+
1024
vi.mock('@/lib/oauth/oauth', () => ({
1125
refreshOAuthToken: vi.fn(),
1226
OAUTH_PROVIDERS: {},
@@ -70,6 +84,7 @@ function mockUpdateChain() {
7084
describe('OAuth Utils', () => {
7185
beforeEach(() => {
7286
vi.clearAllMocks()
87+
capturedLeaderLockOptions.length = 0
7388
__resetCoalesceLocallyForTests()
7489
redisConfigMockFns.mockGetRedisClient.mockReturnValue(null)
7590
redisConfigMockFns.mockAcquireLock.mockResolvedValue(true)
@@ -420,6 +435,62 @@ describe('OAuth Utils', () => {
420435
})
421436
})
422437

438+
describe('QuickBooks refresh locking', () => {
439+
it('keeps the lock and follower wait alive beyond the provider timeout', async () => {
440+
const credential = {
441+
id: 'quickbooks-row',
442+
accessToken: 'expired-token',
443+
refreshToken: 'rotating-refresh-token',
444+
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
445+
providerId: 'quickbooks',
446+
}
447+
mockRefreshOAuthToken.mockResolvedValueOnce({
448+
ok: true,
449+
accessToken: 'new-token',
450+
expiresIn: 3600,
451+
refreshToken: 'new-rotating-refresh-token',
452+
})
453+
mockUpdateChain()
454+
455+
const result = await refreshTokenIfNeeded('request-id', credential, credential.id)
456+
457+
expect(result).toEqual({ accessToken: 'new-token', refreshed: true })
458+
expect(redisConfigMockFns.mockAcquireLock).toHaveBeenCalledWith(
459+
'oauth:refresh:quickbooks-row',
460+
expect.any(String),
461+
30
462+
)
463+
expect(capturedLeaderLockOptions[0]).toMatchObject({ ttlSec: 30, maxWaitMs: 30_000 })
464+
})
465+
466+
it('keeps the default refresh-lock budget for other providers', async () => {
467+
const credential = {
468+
id: 'google-row',
469+
accessToken: 'expired-token',
470+
refreshToken: 'refresh-token',
471+
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
472+
providerId: 'google',
473+
}
474+
mockRefreshOAuthToken.mockResolvedValueOnce({
475+
ok: true,
476+
accessToken: 'new-token',
477+
expiresIn: 3600,
478+
refreshToken: 'new-refresh-token',
479+
})
480+
mockUpdateChain()
481+
482+
await refreshTokenIfNeeded('request-id', credential, credential.id)
483+
484+
expect(redisConfigMockFns.mockAcquireLock).toHaveBeenCalledWith(
485+
'oauth:refresh:google-row',
486+
expect.any(String),
487+
10
488+
)
489+
expect(capturedLeaderLockOptions[0]).not.toHaveProperty('ttlSec')
490+
expect(capturedLeaderLockOptions[0]).not.toHaveProperty('maxWaitMs')
491+
})
492+
})
493+
423494
describe('resolveServiceAccountToken', () => {
424495
it('throws loudly for an unknown provider (never silently attempts Google)', async () => {
425496
await expect(resolveServiceAccountToken('cred-1', 'mystery-provider')).rejects.toThrow(

apps/sim/app/api/auth/oauth/utils.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,8 @@ interface CoalescedRefreshOptions {
684684
*/
685685
const SLACK_LOCK_TTL_SEC = 30
686686
const SLACK_FOLLOWER_MAX_WAIT_MS = SLACK_LOCK_TTL_SEC * 1000
687+
const QUICKBOOKS_LOCK_TTL_SEC = 30
688+
const QUICKBOOKS_FOLLOWER_MAX_WAIT_MS = QUICKBOOKS_LOCK_TTL_SEC * 1000
687689

688690
async function performCoalescedRefresh({
689691
accountId,
@@ -699,6 +701,7 @@ async function performCoalescedRefresh({
699701
* dead-flagged, and written per installation rather than per row.
700702
*/
701703
const slackTeamId = isSlackProvider(providerId) ? extractSlackTeamId(providerAccountId) : null
704+
const isQuickBooks = providerId === 'quickbooks'
702705
const scopeKey = slackTeamId ? `slack:${slackTeamId}` : accountId
703706

704707
const logContext = {
@@ -727,7 +730,14 @@ async function performCoalescedRefresh({
727730
// so their wait and the lock TTL must outlast the 15s provider timeout —
728731
// the 3s/10s defaults would fail followers early and let a second leader
729732
// start a concurrent rotation mid-refresh.
730-
...(slackTeamId ? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC } : {}),
733+
...(slackTeamId
734+
? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC }
735+
: isQuickBooks
736+
? {
737+
maxWaitMs: QUICKBOOKS_FOLLOWER_MAX_WAIT_MS,
738+
ttlSec: QUICKBOOKS_LOCK_TTL_SEC,
739+
}
740+
: {}),
731741
onLeader: async () => {
732742
try {
733743
let refreshTokenToUse = refreshToken

0 commit comments

Comments
 (0)