Skip to content

Commit 5b62952

Browse files
committed
fix(connectors): stop hydration after rate limits
1 parent 5bb8977 commit 5b62952

6 files changed

Lines changed: 279 additions & 49 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { afterEach, describe, expect, it, vi } from 'vitest'
5+
import { githubConnector } from '@/connectors/github/github'
6+
7+
describe('githubConnector.getDocument', () => {
8+
afterEach(() => {
9+
vi.unstubAllGlobals()
10+
})
11+
12+
it('uses the object media type and hydrates large file content through the blob API', async () => {
13+
const fetchMock = vi
14+
.fn()
15+
.mockResolvedValueOnce(
16+
new Response(
17+
JSON.stringify({
18+
sha: 'blob-sha',
19+
size: 2 * 1024 * 1024,
20+
content: '',
21+
encoding: 'none',
22+
}),
23+
{ status: 200, headers: { 'last-modified': 'Fri, 28 Aug 2026 12:00:00 GMT' } }
24+
)
25+
)
26+
.mockResolvedValueOnce(
27+
new Response('large text file', { status: 200, headers: { 'content-length': '15' } })
28+
)
29+
vi.stubGlobal('fetch', fetchMock)
30+
31+
const document = await githubConnector.getDocument(
32+
'token',
33+
{ repository: 'owner/repo', branch: 'main' },
34+
'docs/large.md'
35+
)
36+
37+
expect(fetchMock).toHaveBeenCalledTimes(2)
38+
expect(fetchMock.mock.calls[0][1]).toMatchObject({
39+
headers: expect.objectContaining({ Accept: 'application/vnd.github.object+json' }),
40+
})
41+
expect(fetchMock.mock.calls[1][1]).toMatchObject({
42+
headers: expect.objectContaining({ Accept: 'application/vnd.github.raw+json' }),
43+
})
44+
expect(document).toMatchObject({
45+
externalId: 'docs/large.md',
46+
content: 'large text file',
47+
contentDeferred: false,
48+
contentHash: 'git-sha:blob-sha',
49+
})
50+
})
51+
52+
it('returns null only when a listed path is no longer present', async () => {
53+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 404 })))
54+
55+
await expect(
56+
githubConnector.getDocument('token', { repository: 'owner/repo' }, 'deleted.md')
57+
).resolves.toBeNull()
58+
})
59+
60+
it('records a blob that exceeds the byte cap as a visible skipped document', async () => {
61+
const fetchMock = vi
62+
.fn()
63+
.mockResolvedValueOnce(
64+
new Response(
65+
JSON.stringify({
66+
sha: 'blob-sha',
67+
size: 2 * 1024 * 1024,
68+
content: '',
69+
encoding: 'none',
70+
}),
71+
{ status: 200 }
72+
)
73+
)
74+
.mockResolvedValueOnce(
75+
new Response('oversized', {
76+
status: 200,
77+
headers: { 'content-length': String(100 * 1024 * 1024 + 1) },
78+
})
79+
)
80+
vi.stubGlobal('fetch', fetchMock)
81+
82+
await expect(
83+
githubConnector.getDocument('token', { repository: 'owner/repo' }, 'oversized.md')
84+
).resolves.toMatchObject({
85+
externalId: 'oversized.md',
86+
content: '',
87+
skippedReason: 'File exceeds the 100MB size limit and was not indexed',
88+
})
89+
})
90+
91+
it('surfaces a non-rate-limit 403 as a document failure', async () => {
92+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 403 })))
93+
94+
await expect(
95+
githubConnector.getDocument('token', { repository: 'owner/repo' }, 'private.md')
96+
).rejects.toThrow('Failed to fetch file private.md: 403')
97+
})
98+
})

‎apps/sim/connectors/github/github.ts‎

Lines changed: 20 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import { githubConnectorMeta } from '@/connectors/github/meta'
55
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
66
import {
77
CONNECTOR_MAX_FILE_BYTES,
8+
ConnectorFileTooLargeError,
89
markSkipped,
910
parseTagDate,
11+
readBodyWithLimit,
1012
sizeLimitSkipReason,
1113
stubOrSkipBySize,
1214
takeIndexableWithinCap,
@@ -156,7 +158,7 @@ async function fetchBlobContent(
156158
const response = await fetchWithRetry(url, {
157159
method: 'GET',
158160
headers: {
159-
Accept: 'application/vnd.github+json',
161+
Accept: 'application/vnd.github.raw+json',
160162
Authorization: `Bearer ${accessToken}`,
161163
'X-GitHub-Api-Version': '2022-11-28',
162164
},
@@ -166,25 +168,12 @@ async function fetchBlobContent(
166168
throw new Error(`Failed to fetch git blob ${sha}: ${response.status}`)
167169
}
168170

169-
const data = await response.json()
170-
const content = (data.content as string) || ''
171-
const encoding = data.encoding as string | undefined
172-
173-
if (encoding === 'base64') {
174-
const buf = Buffer.from(content, 'base64')
175-
if (isBinaryBuffer(buf)) return null
176-
return buf.toString('utf8')
171+
const buffer = await readBodyWithLimit(response, MAX_FILE_SIZE)
172+
if (!buffer) {
173+
throw new ConnectorFileTooLargeError(MAX_FILE_SIZE)
177174
}
178-
/**
179-
* `GET /repos/{owner}/{repo}/git/blobs/{sha}` documents a single response
180-
* encoding: "The `content` in the response will always be Base64 encoded."
181-
* The "Currently, `utf-8` and `base64` are supported" sentence belongs to the
182-
* `encoding` REQUEST parameter of `POST .../git/blobs` (Create a blob) and does
183-
* not describe this response, so no `utf-8` branch is warranted here. Any other
184-
* encoding would silently persist empty content, so it throws and surfaces as a
185-
* failed document instead.
186-
*/
187-
throw new Error(`Unexpected git blob encoding for ${sha}: ${encoding ?? 'undefined'}`)
175+
if (isBinaryBuffer(buffer)) return null
176+
return buffer.toString('utf8')
188177
}
189178

190179
/**
@@ -325,33 +314,14 @@ export const githubConnector: ConnectorConfig = {
325314
const response = await fetchWithRetry(url, {
326315
method: 'GET',
327316
headers: {
328-
Accept: 'application/vnd.github+json',
317+
Accept: 'application/vnd.github.object+json',
329318
Authorization: `Bearer ${accessToken}`,
330319
'X-GitHub-Api-Version': '2022-11-28',
331320
},
332321
})
333322

334323
if (!response.ok) {
335324
if (response.status === 404) return null
336-
/**
337-
* A rate-limit 403 never reaches here: `fetchWithRetry` treats a 403 carrying
338-
* `retry-after` or `x-ratelimit-remaining: 0` as retryable and throws once the
339-
* retries are spent, so it lands in the catch below as a failure.
340-
*
341-
* A 403 that survives is usually an authorization denial, but NOT always: this
342-
* request sends `application/vnd.github+json`, and the Contents API documents
343-
* that files between 1-100 MB support "only the `raw` or `object` custom media
344-
* types". A >1 MB text file therefore also lands here and is dropped, which on
345-
* an `add` is silent (a fulfilled `null` records no failure). Reconciliation is
346-
* unaffected — the file is already in `seenExternalIds` from the listing.
347-
*/
348-
if (response.status === 403) {
349-
logger.info('Skipping GitHub file rejected by Contents API', {
350-
path,
351-
status: response.status,
352-
})
353-
return null
354-
}
355325
throw new Error(`Failed to fetch file ${path}: ${response.status}`)
356326
}
357327

@@ -392,15 +362,18 @@ export const githubConnector: ConnectorConfig = {
392362
* "only the `raw` or `object` custom media types are supported", and it is
393363
* specifically "when using the `object` media type" that "the `content` field
394364
* will be an empty string and the `encoding` field will be `none`".
395-
*
396-
* This request sends `application/vnd.github+json`, so that precondition does
397-
* not hold and this branch is currently unreachable — such files 403 above
398-
* instead. Reaching it would require requesting
399-
* `application/vnd.github.object+json`. The fallback itself is correct: the Git
400-
* Blobs API returns the same blob as JSON and is documented to support blobs up
401-
* to 100 MB.
365+
* The Git Blobs fallback streams that same blob through GitHub's documented raw
366+
* media type and supports blobs up to 100 MB.
402367
*/
403-
const blobContent = await fetchBlobContent(accessToken, owner, repo, data.sha as string)
368+
let blobContent: string | null
369+
try {
370+
blobContent = await fetchBlobContent(accessToken, owner, repo, data.sha as string)
371+
} catch (error) {
372+
if (error instanceof ConnectorFileTooLargeError) {
373+
return markSkipped(stub, sizeLimitSkipReason(MAX_FILE_SIZE))
374+
}
375+
throw error
376+
}
404377
if (blobContent === null) {
405378
logger.info('Skipping binary GitHub file', { path, size })
406379
return markSkipped(stub, BINARY_SKIP_REASON)

‎apps/sim/lib/knowledge/connectors/sync-engine.test.ts‎

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ vi.mock('@/background/knowledge-connector-sync', () => ({
5151
knowledgeConnectorSync: { trigger: vi.fn() },
5252
}))
5353

54-
const { mockMapTags, mockListDocuments } = vi.hoisted(() => ({
54+
const { mockGetDocument, mockMapTags, mockListDocuments } = vi.hoisted(() => ({
55+
mockGetDocument: vi.fn(),
5556
mockMapTags: vi.fn(),
5657
mockListDocuments: vi.fn(),
5758
}))
@@ -71,6 +72,7 @@ vi.mock('@/connectors/registry.server', () => ({
7172
paged: {
7273
name: 'Paged',
7374
auth: { mode: 'apiKey', optional: true },
75+
getDocument: mockGetDocument,
7476
listDocuments: mockListDocuments,
7577
},
7678
},
@@ -1166,6 +1168,101 @@ describe('executeSync working-set overflow admission', () => {
11661168
)
11671169
})
11681170

1171+
describe('executeSync deferred hydration rate limits', () => {
1172+
const NOW = new Date('2026-08-29T03:00:00.000Z')
1173+
const CONNECTOR = {
1174+
id: 'c-1',
1175+
knowledgeBaseId: 'kb-1',
1176+
connectorType: 'paged',
1177+
credentialId: null,
1178+
encryptedApiKey: null,
1179+
sourceConfig: {},
1180+
syncMode: 'full',
1181+
syncIntervalMinutes: 1440,
1182+
status: 'active',
1183+
lastSyncAt: null,
1184+
lastSyncDocCount: null,
1185+
consecutiveFailures: 0,
1186+
syncLockToken: null,
1187+
}
1188+
1189+
const deferredDocument = (index: number): ExternalDocument => ({
1190+
externalId: `external-${index}`,
1191+
title: `Document ${index}`,
1192+
content: '',
1193+
contentDeferred: true,
1194+
contentHash: `hash-${index}`,
1195+
mimeType: 'text/plain',
1196+
metadata: { size: 1024 },
1197+
})
1198+
1199+
beforeEach(() => {
1200+
vi.clearAllMocks()
1201+
resetDbChainMock()
1202+
vi.useFakeTimers()
1203+
vi.setSystemTime(NOW)
1204+
queueTableRows(schemaMock.knowledgeConnector, [CONNECTOR])
1205+
queueTableRows(schemaMock.knowledgeBase, [{ userId: 'u-1', workspaceId: 'ws-1' }])
1206+
queueTableRows(schemaMock.document, [])
1207+
queueTableRows(schemaMock.document, [])
1208+
queueTableRows(schemaMock.document, [])
1209+
queueTableRows(schemaMock.document, [])
1210+
queueTableRows(schemaMock.knowledgeConnector, [
1211+
{ connectorArchivedAt: null, connectorDeletedAt: null, kbDeletedAt: null },
1212+
])
1213+
dbChainMockFns.returning.mockResolvedValueOnce([CONNECTOR])
1214+
})
1215+
1216+
afterEach(() => {
1217+
vi.useRealTimers()
1218+
})
1219+
1220+
it('stops after the active batch and preserves the provider retry delay', async () => {
1221+
const { executeSync } = await import('@/lib/knowledge/connectors/sync-engine')
1222+
const documents = Array.from({ length: 6 }, (_, index) => deferredDocument(index))
1223+
const rateLimitError = Object.assign(new Error('HTTP 403 - upstream rate limit exceeded'), {
1224+
status: 403,
1225+
headers: new Headers({ 'x-ratelimit-remaining': '0' }),
1226+
retryAfterMs: 45 * 60 * 1000,
1227+
})
1228+
1229+
mockListDocuments.mockResolvedValue({ documents, hasMore: false })
1230+
mockGetDocument.mockImplementation(async (_token, _config, externalId: string) => {
1231+
if (externalId === 'external-2') throw rateLimitError
1232+
return {
1233+
...documents[Number(externalId.slice('external-'.length))],
1234+
content: 'hydrated',
1235+
contentDeferred: false,
1236+
}
1237+
})
1238+
1239+
const result = await executeSync('c-1', {
1240+
billingAttribution: { workspaceId: 'ws-1' } as never,
1241+
})
1242+
1243+
expect(mockGetDocument).toHaveBeenCalledTimes(5)
1244+
expect(mockGetDocument).not.toHaveBeenCalledWith(
1245+
expect.anything(),
1246+
expect.anything(),
1247+
'external-5',
1248+
expect.anything()
1249+
)
1250+
expect(result).toMatchObject({
1251+
docsAdded: 0,
1252+
docsFailed: 0,
1253+
error: rateLimitError.message,
1254+
})
1255+
expect(mockUploadFile).not.toHaveBeenCalled()
1256+
expect(mockProcessDocumentsWithQueue).not.toHaveBeenCalled()
1257+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
1258+
expect.objectContaining({
1259+
status: 'error',
1260+
nextSyncAt: new Date(NOW.getTime() + 45 * 60 * 1000),
1261+
})
1262+
)
1263+
})
1264+
})
1265+
11691266
describe('classifySuspectListing', () => {
11701267
it('trusts a healthy listing', () => {
11711268
expect(classifySuspectListing(100, 100)).toBeNull()

‎apps/sim/lib/knowledge/connectors/sync-engine.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import {
5353
MAX_PROCESSING_ATTEMPTS,
5454
QUEUED_DISPATCH_GRACE_MS,
5555
} from '@/lib/knowledge/documents/types'
56-
import { getRetryAfterMs } from '@/lib/knowledge/documents/utils'
56+
import { getRetryAfterMs, isRateLimitError } from '@/lib/knowledge/documents/utils'
5757
import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service'
5858
import { StorageService } from '@/lib/uploads'
5959
import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key'
@@ -2443,6 +2443,14 @@ export async function executeSync(
24432443
})
24442444
)
24452445

2446+
const rateLimitFailure = hydrated.find(
2447+
(outcome): outcome is PromiseRejectedResult =>
2448+
outcome.status === 'rejected' && isRateLimitError(outcome.reason)
2449+
)
2450+
if (rateLimitFailure) {
2451+
throw rateLimitFailure.reason
2452+
}
2453+
24462454
for (let i = 0; i < hydrated.length; i++) {
24472455
const outcome = hydrated[i]
24482456
if (outcome.status === 'fulfilled' && outcome.value) {

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
getRetryAfterMs,
1818
type HTTPError,
1919
hasRateLimitEvidence,
20+
isRateLimitError,
2021
isRetryableError,
2122
readBoundedHttpErrorPayload,
2223
resolveRetryDelayMs,
@@ -640,6 +641,34 @@ describe('getRetryAfterMs', () => {
640641
)
641642
})
642643

644+
describe('isRateLimitError', () => {
645+
it('accepts a 429 without requiring response headers', () => {
646+
expect(isRateLimitError(Object.assign(new Error('throttled'), { status: 429 }))).toBe(true)
647+
})
648+
649+
it('accepts a GitHub 403 only with structured rate-limit evidence', () => {
650+
expect(
651+
isRateLimitError(
652+
Object.assign(new Error('forbidden'), {
653+
status: 403,
654+
headers: headers({ 'x-ratelimit-remaining': '0' }),
655+
})
656+
)
657+
).toBe(true)
658+
expect(isRateLimitError(Object.assign(new Error('forbidden'), { status: 403 }))).toBe(false)
659+
})
660+
661+
it('finds a structured rate-limit rejection through an error cause chain', () => {
662+
const providerError = Object.assign(new Error('throttled'), { status: 429 })
663+
expect(isRateLimitError(new Error('hydration failed', { cause: providerError }))).toBe(true)
664+
})
665+
666+
it('does not classify retryable text or transient HTTP failures as provider throttling', () => {
667+
expect(isRateLimitError(new Error('rate limit exceeded'))).toBe(false)
668+
expect(isRateLimitError(Object.assign(new Error('unavailable'), { status: 503 }))).toBe(false)
669+
})
670+
})
671+
643672
describe('retryWithExponentialBackoff retry budget', () => {
644673
afterEach(() => {
645674
vi.useRealTimers()

0 commit comments

Comments
 (0)