Skip to content

Commit 24ec909

Browse files
committed
fix(connectors): validate retry response lifecycles
1 parent fd270d8 commit 24ec909

4 files changed

Lines changed: 62 additions & 6 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,18 @@ describe('resolveAtlassianCloudId', () => {
217217
'Invalid Jira accessible-resources response'
218218
)
219219
})
220+
221+
it.each([
222+
[{ url: SITE }],
223+
[{ id: CLOUD_ID }],
224+
[{ id: '', url: SITE }],
225+
[{ id: CLOUD_ID, url: '' }],
226+
[null],
227+
])('rejects malformed resource entries in an otherwise valid array', async (resources) => {
228+
fetchMock.mockResolvedValue(createMockResponse({ json: resources }))
229+
230+
await expect(resolveAtlassianCloudId(options())).rejects.toThrow(
231+
'Invalid Jira accessible-resources response'
232+
)
233+
})
220234
})

apps/sim/lib/atlassian/discovery.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ interface AccessibleResource {
102102
url: string
103103
}
104104

105+
function isAccessibleResource(value: unknown): value is AccessibleResource {
106+
if (typeof value !== 'object' || value === null) return false
107+
const resource = value as Record<string, unknown>
108+
return (
109+
typeof resource.id === 'string' &&
110+
resource.id.trim().length > 0 &&
111+
typeof resource.url === 'string' &&
112+
resource.url.trim().length > 0
113+
)
114+
}
115+
105116
interface ResolveAtlassianCloudIdOptions {
106117
domain: string
107118
accessToken: string
@@ -203,7 +214,7 @@ export function selectAtlassianCloudId(
203214
domain: string,
204215
product: string
205216
): string {
206-
if (!Array.isArray(resources)) {
217+
if (!Array.isArray(resources) || !resources.every(isAccessibleResource)) {
207218
throw new Error(`Invalid ${product} accessible-resources response`)
208219
}
209220

@@ -215,16 +226,14 @@ export function selectAtlassianCloudId(
215226
}
216227

217228
const siteUrl = normalizeAtlassianSiteUrl(domain)
218-
const match = (resources as AccessibleResource[]).find(
219-
(r) => normalizeAtlassianSiteUrl(r.url) === siteUrl
220-
)
229+
const match = resources.find((r) => normalizeAtlassianSiteUrl(r.url) === siteUrl)
221230
if (match) return match.id
222231

223-
if (resources.length === 1) return (resources as AccessibleResource[])[0].id
232+
if (resources.length === 1) return resources[0].id
224233

225234
throw new Error(
226235
`Could not match ${product} domain "${domain}" to any accessible resource. ` +
227-
`Available sites: ${(resources as AccessibleResource[]).map((r) => r.url).join(', ')}`
236+
`Available sites: ${resources.map((r) => r.url).join(', ')}`
228237
)
229238
}
230239

apps/sim/lib/knowledge/documents/utils.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,26 @@ describe('fetchWithRetry rate-limit handling', () => {
547547
expect(fetchMock).toHaveBeenCalledTimes(1)
548548
})
549549

550+
it('cancels an omitted rate-limit response body before throwing', async () => {
551+
let cancelled = false
552+
const body = new ReadableStream<Uint8Array>({
553+
cancel() {
554+
cancelled = true
555+
},
556+
})
557+
globalThis.fetch = vi.fn().mockResolvedValue(
558+
new Response(body, {
559+
status: 429,
560+
headers: { 'retry-after': '900' },
561+
})
562+
)
563+
564+
await expect(
565+
fetchWithRetry('https://api.github.com/repos', {}, { ...FAST_RETRY, maxRetries: 0 })
566+
).rejects.toThrow('HTTP 429 - upstream rate limit exceeded')
567+
expect(cancelled).toBe(true)
568+
})
569+
550570
it('waits until an admitted x-rate-limit-reset instant before retrying', async () => {
551571
vi.useFakeTimers()
552572
const now = 1_700_000_000_000

apps/sim/lib/knowledge/documents/utils.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,16 @@ interface RetryableHttpResponse {
286286
text?: () => Promise<string>
287287
}
288288

289+
/** Releases a response stream when its provider-controlled body is intentionally omitted. */
290+
async function cancelHttpResponseBody(response: RetryableHttpResponse): Promise<void> {
291+
if (!response.body) return
292+
try {
293+
await response.body.cancel()
294+
} catch {
295+
return
296+
}
297+
}
298+
289299
/**
290300
* Builds the bounded error shared by direct and SSRF-safe connector fetches.
291301
* Rate-limit responses are named from trusted status/header evidence while all
@@ -296,6 +306,9 @@ export async function createRetryableHttpError(
296306
): Promise<HTTPError> {
297307
const rateLimited =
298308
response.status === 429 || (response.status === 403 && hasRateLimitEvidence(response.headers))
309+
if (rateLimited) {
310+
await cancelHttpResponseBody(response)
311+
}
299312
const diagnostic = rateLimited
300313
? 'upstream rate limit exceeded'
301314
: await readBoundedHttpErrorBody(response)

0 commit comments

Comments
 (0)