Skip to content

Commit bcc3899

Browse files
waleedlatif1waleedlatif1
committed
fix(atlassian): share one cached, retrying cloudId resolver across Jira, Confluence, and JSM
Every Jira, Confluence, and JSM tool re-resolved its site `cloudId` from `accessible-resources` on each invocation, so a run touching several Atlassian blocks paid a round trip per block and failed outright if any one of them caught a transient fault. A single Atlassian 500 took down a production run this way: the shared retry predicate covers 429/502/503/504 but not 500, so the call was never replayed. Four hand-rolled copies of that lookup now read through one memoized resolver. It caches the promise rather than the value, so concurrent callers join a lookup already in flight and a rejection is evicted instead of pinned for the TTL. Only an exact domain match is retained — a single-site fallback is a property of the calling token, not of the domain, so it answers its own caller without answering the next one. Discovery is an idempotent GET, so it replays transient 5xx. That is scoped here rather than widened into the shared predicate, which also guards non-idempotent writes; it also keeps the failure out of a whole-block replay, which would re-run a write a JSM block had already performed. The budget is tighter than the shared ~31s default — four attempts across ~3.5s — and the request carries a timeout so a wedged fetch cannot strand the callers joined to it. Two defects fall out of the consolidation. `getConfluenceCloudId` never checked the response status, so a 500 parsed as JSON, failed the array check, and surfaced as `No Confluence resources found` — pointing at site permissions rather than the transient fault. `getAssetsWorkspaceId` used a bare fetch with no retry and no cache, leaving the Assets path with two uncached discovery hops.
1 parent f8644cc commit bcc3899

6 files changed

Lines changed: 461 additions & 131 deletions

File tree

‎apps/sim/lib/atlassian/discovery.test.ts‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockResponse } from '@sim/testing'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
import {
7+
clearAtlassianCloudIdCache,
8+
normalizeAtlassianSiteUrl,
9+
resolveAtlassianCloudId,
10+
} from '@/lib/atlassian/discovery'
11+
12+
const SITE = 'https://acme.atlassian.net'
13+
const CLOUD_ID = 'cloud-abc'
14+
15+
/** Options for the site under test; override only the field a case is exercising. */
16+
function options(over: Record<string, unknown> = {}) {
17+
return { domain: 'acme.atlassian.net', accessToken: 't', product: 'Jira', ...over } as Parameters<
18+
typeof resolveAtlassianCloudId
19+
>[0]
20+
}
21+
22+
/** Tiny delays so retry cases do not spend real seconds sleeping. */
23+
const FAST = { initialDelayMs: 1, maxDelayMs: 1 }
24+
25+
function sites(entries: Array<{ id: string; url: string }>) {
26+
return createMockResponse({ json: entries })
27+
}
28+
29+
function failure(status: number, body: unknown = { key: 'unexpectedError' }) {
30+
return createMockResponse({ status, json: body })
31+
}
32+
33+
let fetchMock: ReturnType<typeof vi.fn>
34+
35+
beforeEach(() => {
36+
clearAtlassianCloudIdCache()
37+
fetchMock = vi.fn()
38+
vi.stubGlobal('fetch', fetchMock)
39+
})
40+
41+
afterEach(() => {
42+
vi.unstubAllGlobals()
43+
vi.restoreAllMocks()
44+
})
45+
46+
describe('normalizeAtlassianSiteUrl', () => {
47+
it.each([
48+
['acme.atlassian.net', SITE],
49+
['https://acme.atlassian.net', SITE],
50+
['http://ACME.atlassian.net/', SITE],
51+
[' acme.atlassian.net// ', SITE],
52+
])('normalizes %s', (input, expected) => {
53+
expect(normalizeAtlassianSiteUrl(input)).toBe(expected)
54+
})
55+
})
56+
57+
describe('resolveAtlassianCloudId', () => {
58+
it('resolves an exact domain match', async () => {
59+
fetchMock.mockResolvedValue(sites([{ id: CLOUD_ID, url: SITE }]))
60+
61+
await expect(resolveAtlassianCloudId(options())).resolves.toBe(CLOUD_ID)
62+
})
63+
64+
it('serves a repeat lookup from cache without a second request', async () => {
65+
fetchMock.mockResolvedValue(sites([{ id: CLOUD_ID, url: SITE }]))
66+
67+
await resolveAtlassianCloudId(options())
68+
await resolveAtlassianCloudId(options())
69+
70+
expect(fetchMock).toHaveBeenCalledTimes(1)
71+
})
72+
73+
it('collapses concurrent lookups into one request', async () => {
74+
fetchMock.mockResolvedValue(sites([{ id: CLOUD_ID, url: SITE }]))
75+
76+
const resolved = await Promise.all([
77+
resolveAtlassianCloudId(options()),
78+
resolveAtlassianCloudId(options()),
79+
resolveAtlassianCloudId(options()),
80+
])
81+
82+
expect(resolved).toEqual([CLOUD_ID, CLOUD_ID, CLOUD_ID])
83+
expect(fetchMock).toHaveBeenCalledTimes(1)
84+
})
85+
86+
it.each([500, 503, 507])(
87+
'retries a %i and succeeds, instead of failing the call',
88+
async (status) => {
89+
fetchMock
90+
.mockResolvedValueOnce(failure(status))
91+
.mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }]))
92+
93+
await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).resolves.toBe(CLOUD_ID)
94+
}
95+
)
96+
97+
it('keeps the transient-5xx condition when a caller tunes the retry budget', async () => {
98+
fetchMock
99+
.mockResolvedValueOnce(failure(500))
100+
.mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }]))
101+
102+
// Shaped like VALIDATE_RETRY_OPTIONS: counts only, no retryCondition.
103+
await expect(
104+
resolveAtlassianCloudId(options({ retryOptions: { maxRetries: 3, ...FAST } }))
105+
).resolves.toBe(CLOUD_ID)
106+
})
107+
108+
it('gives up on a persistent fault within a bounded attempt budget', async () => {
109+
fetchMock.mockImplementation(async () => failure(500))
110+
111+
// Delays only. `maxRetries` still comes from the discovery budget, so this
112+
// fails if the shared default of 5 ever leaks back in.
113+
await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow(
114+
/Failed to fetch Jira accessible resources: 500/
115+
)
116+
expect(fetchMock).toHaveBeenCalledTimes(4)
117+
})
118+
119+
it('does not retry a client error', async () => {
120+
fetchMock.mockResolvedValue(failure(403, { message: 'nope' }))
121+
122+
await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow(
123+
/Failed to fetch Jira accessible resources: 403/
124+
)
125+
expect(fetchMock).toHaveBeenCalledTimes(1)
126+
})
127+
128+
it('does not pin a failure in the cache', async () => {
129+
fetchMock.mockResolvedValueOnce(failure(403, { message: 'nope' }))
130+
await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow()
131+
132+
fetchMock.mockResolvedValueOnce(sites([{ id: CLOUD_ID, url: SITE }]))
133+
await expect(resolveAtlassianCloudId(options())).resolves.toBe(CLOUD_ID)
134+
expect(fetchMock).toHaveBeenCalledTimes(2)
135+
})
136+
137+
it('surfaces a non-OK status rather than reporting no resources', async () => {
138+
fetchMock.mockImplementation(async () => failure(500))
139+
140+
await expect(
141+
resolveAtlassianCloudId(options({ product: 'Confluence', retryOptions: FAST }))
142+
).rejects.toThrow(/Failed to fetch Confluence accessible resources: 500/)
143+
})
144+
145+
it('does not retain a single-site fallback, since it is token-specific', async () => {
146+
fetchMock.mockImplementation(async () =>
147+
sites([{ id: 'other-cloud', url: 'https://other.atlassian.net' }])
148+
)
149+
150+
await expect(resolveAtlassianCloudId(options())).resolves.toBe('other-cloud')
151+
await resolveAtlassianCloudId(options())
152+
153+
expect(fetchMock).toHaveBeenCalledTimes(2)
154+
})
155+
156+
it('rejects rather than throwing synchronously on a missing domain', async () => {
157+
const call = resolveAtlassianCloudId(options({ domain: undefined }))
158+
159+
await expect(call).rejects.toThrow()
160+
expect(fetchMock).not.toHaveBeenCalled()
161+
})
162+
163+
it('reports the available sites when several are accessible and none match', async () => {
164+
fetchMock.mockResolvedValue(
165+
sites([
166+
{ id: 'a', url: 'https://one.atlassian.net' },
167+
{ id: 'b', url: 'https://two.atlassian.net' },
168+
])
169+
)
170+
171+
await expect(resolveAtlassianCloudId(options())).rejects.toThrow(
172+
/Available sites: https:\/\/one.atlassian.net, https:\/\/two.atlassian.net/
173+
)
174+
})
175+
176+
it('rejects when the token can see no sites', async () => {
177+
fetchMock.mockResolvedValue(sites([]))
178+
179+
await expect(resolveAtlassianCloudId(options())).rejects.toThrow('No Jira resources found')
180+
})
181+
})

0 commit comments

Comments
 (0)