Skip to content

Commit 2c64932

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

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
@@ -190,6 +190,25 @@ describe('OAuth Utils', () => {
190190
).rejects.toThrow('invalid_grant (google: Token has been expired or revoked.)')
191191
})
192192

193+
it('should throw the generic error on transient failures without a provider code', async () => {
194+
const mockCredential = {
195+
id: 'credential-id',
196+
accessToken: 'expired-token',
197+
refreshToken: 'refresh-token',
198+
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
199+
providerId: 'google',
200+
}
201+
202+
mockRefreshOAuthToken.mockResolvedValueOnce({
203+
ok: false,
204+
message: 'fetch failed',
205+
})
206+
207+
await expect(
208+
refreshTokenIfNeeded('request-id', mockCredential, 'credential-id')
209+
).rejects.toThrow('Failed to refresh token')
210+
})
211+
193212
it('should not attempt refresh if no refresh token', async () => {
194213
const mockCredential = {
195214
id: 'credential-id',
@@ -349,6 +368,7 @@ describe('OAuth Utils', () => {
349368
expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][0]).toBe(
350369
'oauth:refresh:slack:T08CM6ZNYBE'
351370
)
371+
expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][2]).toBe(20)
352372
expect(mockRefreshOAuthToken).toHaveBeenCalledWith('slack', 'live-rt')
353373
expect(mockSet).toHaveBeenCalledWith(
354374
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
@@ -677,6 +677,15 @@ interface CoalescedRefreshOutcome {
677677
error?: OAuthRefreshError
678678
}
679679

680+
/**
681+
* Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in
682+
* lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request
683+
* a follower of one refresh, so followers must keep polling for the leader's
684+
* full provider window and the lock must not expire under a live refresh.
685+
*/
686+
const SLACK_FOLLOWER_MAX_WAIT_MS = 16_000
687+
const SLACK_LOCK_TTL_SEC = 20
688+
680689
async function performCoalescedRefresh({
681690
accountId,
682691
providerId,
@@ -715,6 +724,11 @@ async function performCoalescedRefresh({
715724
const refreshPromise = coalesceLocally(lockKey, () =>
716725
withLeaderLock<CoalescedRefreshOutcome>({
717726
key: lockKey,
727+
// Installation-keyed Slack locks gather followers from every sibling row,
728+
// so their wait and the lock TTL must outlast the 15s provider timeout —
729+
// the 3s/10s defaults would fail followers early and let a second leader
730+
// start a concurrent rotation mid-refresh.
731+
...(slackTeamId ? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC } : {}),
718732
onLeader: async () => {
719733
try {
720734
let refreshTokenToUse = refreshToken
@@ -753,7 +767,17 @@ async function performCoalescedRefresh({
753767
}
754768
return {
755769
accessToken: null,
756-
error: new OAuthRefreshError(providerId, result.errorCode, result.errorDescription),
770+
// No errorCode = transient (timeout/network), not a provider rejection —
771+
// stay errorless so callers keep their null-fallback behavior.
772+
...(result.errorCode
773+
? {
774+
error: new OAuthRefreshError(
775+
providerId,
776+
result.errorCode,
777+
result.errorDescription
778+
),
779+
}
780+
: {}),
757781
}
758782
}
759783

0 commit comments

Comments
 (0)