Skip to content

Commit aa7af2f

Browse files
fix(oauth): keep transient refresh failures errorless and size slack lock budgets past the provider timeout
1 parent ededf84 commit aa7af2f

2 files changed

Lines changed: 45 additions & 1 deletion

File tree

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,25 @@ describe('OAuth Utils', () => {
183183
).rejects.toThrow('invalid_grant (google: Token has been expired or revoked.)')
184184
})
185185

186+
it('should throw the generic error on transient failures without a provider code', async () => {
187+
const mockCredential = {
188+
id: 'credential-id',
189+
accessToken: 'expired-token',
190+
refreshToken: 'refresh-token',
191+
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
192+
providerId: 'google',
193+
}
194+
195+
mockRefreshOAuthToken.mockResolvedValueOnce({
196+
ok: false,
197+
message: 'fetch failed',
198+
})
199+
200+
await expect(
201+
refreshTokenIfNeeded('request-id', mockCredential, 'credential-id')
202+
).rejects.toThrow('Failed to refresh token')
203+
})
204+
186205
it('should not attempt refresh if no refresh token', async () => {
187206
const mockCredential = {
188207
id: 'credential-id',
@@ -342,6 +361,7 @@ describe('OAuth Utils', () => {
342361
expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][0]).toBe(
343362
'oauth:refresh:slack:T08CM6ZNYBE'
344363
)
364+
expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][2]).toBe(20)
345365
expect(mockRefreshOAuthToken).toHaveBeenCalledWith('slack', 'live-rt')
346366
expect(mockSet).toHaveBeenCalledWith(
347367
expect.objectContaining({ accessToken: 'new-at', refreshToken: 'new-rt' })

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,15 @@ interface CoalescedRefreshOutcome {
503503
error?: OAuthRefreshError
504504
}
505505

506+
/**
507+
* Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in
508+
* lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request
509+
* a follower of one refresh, so followers must keep polling for the leader's
510+
* full provider window and the lock must not expire under a live refresh.
511+
*/
512+
const SLACK_FOLLOWER_MAX_WAIT_MS = 16_000
513+
const SLACK_LOCK_TTL_SEC = 20
514+
506515
async function performCoalescedRefresh({
507516
accountId,
508517
providerId,
@@ -541,6 +550,11 @@ async function performCoalescedRefresh({
541550
const refreshPromise = coalesceLocally(lockKey, () =>
542551
withLeaderLock<CoalescedRefreshOutcome>({
543552
key: lockKey,
553+
// Installation-keyed Slack locks gather followers from every sibling row,
554+
// so their wait and the lock TTL must outlast the 15s provider timeout —
555+
// the 3s/10s defaults would fail followers early and let a second leader
556+
// start a concurrent rotation mid-refresh.
557+
...(slackTeamId ? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC } : {}),
544558
onLeader: async () => {
545559
try {
546560
let refreshTokenToUse = refreshToken
@@ -579,7 +593,17 @@ async function performCoalescedRefresh({
579593
}
580594
return {
581595
accessToken: null,
582-
error: new OAuthRefreshError(providerId, result.errorCode, result.errorDescription),
596+
// No errorCode = transient (timeout/network), not a provider rejection —
597+
// stay errorless so callers keep their null-fallback behavior.
598+
...(result.errorCode
599+
? {
600+
error: new OAuthRefreshError(
601+
providerId,
602+
result.errorCode,
603+
result.errorDescription
604+
),
605+
}
606+
: {}),
583607
}
584608
}
585609

0 commit comments

Comments
 (0)