Skip to content

Commit 94d432a

Browse files
committed
fix(apify): give the sync runs headroom, route polling through the transport
Renaming the reserved timeout param in this branch removed the only thing that was setting a fetch deadline for Apify, so every sync run began riding the bare 300000ms default -- and Apify's sync endpoint returns 408 at exactly 300s. A run near the boundary was a coin flip between Apify's structured 408 and Sim's generic timeout, which loses the provider's diagnosis. The two sync operations now get a deliberate 330s transport deadline, mirroring the 30s headroom the executor already applies to internal routes. run_actor_async polled with bare global fetch, so the dataset read bypassed the response-size cap entirely. It now re-enters executeTool -- the pattern luma and google_docs already use -- reusing apify_get_run and apify_get_dataset_items rather than new request code, so the read is byte-bounded. The loop also checked status before its first sleep, and its exhaustion message no longer hardcodes a duration that the configurable ceiling can contradict. An explicit 0 was dropped by a truthy guard while Apify documents 0 as 'no timeout', so an unbounded run was inexpressible. Note the value that was being dropped is a numeric 0 from a block-output reference; a subBlock string '0' was always truthy. run_actor_sync fabricated runId: 'sync-execution'. The endpoint returns only pagination headers and no run id, so any downstream get_run wired to it was a guaranteed 404. The field is omitted rather than invented. Two limits remain and are documented in the code: the agent path bypasses the block mapper, and executeNestedTool does not forward the abort signal.
1 parent 057ca4e commit 94d432a

6 files changed

Lines changed: 319 additions & 57 deletions

File tree

apps/sim/blocks/blocks/apify.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import { ApifyIcon } from '@/components/icons'
22
import type { BlockConfig, BlockMeta } from '@/blocks/types'
33
import { AuthMode, IntegrationType } from '@/blocks/types'
4-
import type { RunActorResult } from '@/tools/apify/types'
4+
import { APIFY_SYNC_TRANSPORT_TIMEOUT_MS, type RunActorResult } from '@/tools/apify/types'
55

66
const RUN_OPERATIONS = ['apify_run_actor_sync', 'apify_run_actor_async']
77
const RUN_OR_TASK_OPERATIONS = [...RUN_OPERATIONS, 'apify_run_task']
88

9+
/** Operations that call a `run-sync-get-dataset-items` endpoint and inherit its 300s/408 contract. */
10+
const SYNC_RUN_OPERATIONS = new Set(['apify_run_actor_sync', 'apify_run_task'])
11+
912
export const ApifyBlock: BlockConfig<RunActorResult> = {
1013
type: 'apify',
1114
name: 'Apify',
@@ -234,10 +237,18 @@ Return ONLY the valid JSON object - no explanations, no markdown.`,
234237
/**
235238
* `timeout` is reserved by the tool request transport as the outbound fetch
236239
* deadline in milliseconds, so the seconds-valued subBlock is remapped onto a
237-
* tool-specific parameter and cleared here to keep it off the transport.
240+
* tool-specific parameter and never forwarded as-is.
241+
*
242+
* The sync endpoints then get a deliberate transport deadline of their own:
243+
* without one they ride the transport's bare 300000ms fallback, which is the
244+
* same number as Apify's documented 300s sync cap, so a boundary run races a
245+
* structured 408 against a generic transport timeout. See
246+
* `APIFY_SYNC_TRANSPORT_TIMEOUT_MS`.
238247
*/
239-
result.timeout = undefined
240-
if (rest.timeout) {
248+
result.timeout = SYNC_RUN_OPERATIONS.has(operation)
249+
? APIFY_SYNC_TRANSPORT_TIMEOUT_MS
250+
: undefined
251+
if (rest.timeout != null && rest.timeout !== '') {
241252
const timeoutSeconds = Number(rest.timeout)
242253
if (operation === 'apify_run_task') result.taskTimeout = timeoutSeconds
243254
else result.actorTimeout = timeoutSeconds

apps/sim/tools/apify/apify.test.ts

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { DEFAULT_EXECUTION_TIMEOUT_MS } from '@/lib/core/execution-limits'
6+
import { ApifyBlock } from '@/blocks/blocks/apify'
7+
import { apifyRunActorAsyncTool } from '@/tools/apify/run_actor_async'
8+
import { apifyRunActorSyncTool } from '@/tools/apify/run_actor_sync'
9+
import { apifyRunTaskTool } from '@/tools/apify/run_task'
10+
import { APIFY_SYNC_TRANSPORT_TIMEOUT_MS } from '@/tools/apify/types'
11+
import type { ToolConfig, ToolResponse } from '@/tools/types'
12+
13+
vi.mock('@sim/utils/helpers', () => ({
14+
sleep: vi.fn(async () => {}),
15+
}))
16+
17+
function mapBlockParams(params: Record<string, unknown>): Record<string, unknown> {
18+
const build = ApifyBlock.tools.config?.params
19+
if (!build) throw new Error('apify tools.config.params is not defined')
20+
return build(params) as Record<string, unknown>
21+
}
22+
23+
function buildUrl(tool: ToolConfig<any, any>, params: Record<string, unknown>): string {
24+
const { url } = tool.request
25+
return typeof url === 'function' ? url(params as any) : url
26+
}
27+
28+
function jsonResponse(body: unknown): Response {
29+
return {
30+
ok: true,
31+
json: async () => body,
32+
} as unknown as Response
33+
}
34+
35+
describe('apify sync transport deadline', () => {
36+
it('sits above the 300s Apify sync cap so the documented 408 wins the race', () => {
37+
expect(APIFY_SYNC_TRANSPORT_TIMEOUT_MS).toBeGreaterThan(300_000)
38+
})
39+
40+
it('arms a deliberate transport deadline for run_actor_sync', () => {
41+
const result = mapBlockParams({
42+
operation: 'apify_run_actor_sync',
43+
apiKey: 'k',
44+
actorId: 'me/actor',
45+
})
46+
expect(result.timeout).toBe(APIFY_SYNC_TRANSPORT_TIMEOUT_MS)
47+
})
48+
49+
it('arms a deliberate transport deadline for run_task', () => {
50+
const result = mapBlockParams({ operation: 'apify_run_task', apiKey: 'k', taskId: 'me/task' })
51+
expect(result.timeout).toBe(APIFY_SYNC_TRANSPORT_TIMEOUT_MS)
52+
})
53+
54+
it('never derives the transport deadline from the user-facing seconds value', () => {
55+
const result = mapBlockParams({
56+
operation: 'apify_run_actor_sync',
57+
apiKey: 'k',
58+
actorId: 'me/actor',
59+
timeout: '3600',
60+
})
61+
expect(result.actorTimeout).toBe(3600)
62+
expect(result.timeout).toBe(APIFY_SYNC_TRANSPORT_TIMEOUT_MS)
63+
expect(result.timeout).not.toBe(3600)
64+
expect(result.timeout).not.toBe(3_600_000)
65+
})
66+
67+
it('leaves non-sync operations on no transport deadline', () => {
68+
for (const operation of ['apify_run_actor_async', 'apify_get_run', 'apify_get_dataset_items']) {
69+
const result = mapBlockParams({ operation, apiKey: 'k', actorId: 'a', runId: 'r' })
70+
expect(Object.hasOwn(result, 'timeout')).toBe(true)
71+
expect(result.timeout).toBeUndefined()
72+
}
73+
})
74+
})
75+
76+
describe('apify explicit zero timeout', () => {
77+
it('forwards timeout=0 (Apify: no timeout) on every run tool', () => {
78+
expect(buildUrl(apifyRunActorSyncTool, { actorId: 'me/actor', actorTimeout: 0 })).toContain(
79+
'timeout=0'
80+
)
81+
expect(buildUrl(apifyRunActorAsyncTool, { actorId: 'me/actor', actorTimeout: 0 })).toContain(
82+
'timeout=0'
83+
)
84+
expect(buildUrl(apifyRunTaskTool, { taskId: 'me/task', taskTimeout: 0 })).toContain('timeout=0')
85+
})
86+
87+
it('maps an explicit 0 subBlock value through the block mapper', () => {
88+
for (const zero of ['0', 0]) {
89+
expect(
90+
mapBlockParams({
91+
operation: 'apify_run_actor_sync',
92+
apiKey: 'k',
93+
actorId: 'me/actor',
94+
timeout: zero,
95+
}).actorTimeout
96+
).toBe(0)
97+
expect(
98+
mapBlockParams({
99+
operation: 'apify_run_task',
100+
apiKey: 'k',
101+
taskId: 'me/task',
102+
timeout: zero,
103+
}).taskTimeout
104+
).toBe(0)
105+
}
106+
})
107+
108+
it('still drops an untouched (empty string) subBlock value', () => {
109+
const result = mapBlockParams({
110+
operation: 'apify_run_actor_sync',
111+
apiKey: 'k',
112+
actorId: 'me/actor',
113+
timeout: '',
114+
})
115+
expect(result.actorTimeout).toBeUndefined()
116+
expect(result.taskTimeout).toBeUndefined()
117+
})
118+
})
119+
120+
describe('apify run_actor_sync response contract', () => {
121+
it('emits no fabricated run id — the sync endpoint returns none', async () => {
122+
const result = await apifyRunActorSyncTool.transformResponse!(jsonResponse([{ a: 1 }]))
123+
expect(Object.hasOwn(result.output, 'runId')).toBe(false)
124+
expect(result.output.items).toEqual([{ a: 1 }])
125+
expect(apifyRunActorSyncTool.outputs?.runId).toBeUndefined()
126+
})
127+
128+
it('guards a non-array response body', async () => {
129+
const result = await apifyRunActorSyncTool.transformResponse!(jsonResponse({ error: 'nope' }))
130+
expect(result.output.items).toEqual([])
131+
})
132+
})
133+
134+
describe('apify run_actor_async polling', () => {
135+
const fetchSpy = vi.fn(() => {
136+
throw new Error('global fetch must not be used inside postProcess')
137+
})
138+
139+
beforeEach(() => {
140+
vi.clearAllMocks()
141+
vi.stubGlobal('fetch', fetchSpy)
142+
})
143+
144+
afterEach(() => {
145+
vi.unstubAllGlobals()
146+
})
147+
148+
function startedResult(): ToolResponse {
149+
return {
150+
success: true,
151+
output: { success: true, runId: 'run-1', status: 'RUNNING' },
152+
}
153+
}
154+
155+
it('routes both follow-up requests through the guarded tool transport', async () => {
156+
const executeTool = vi.fn(async (toolId: string) => {
157+
if (toolId === 'apify_get_run') {
158+
return {
159+
success: true,
160+
output: { success: true, runId: 'run-1', status: 'SUCCEEDED', datasetId: 'ds-1' },
161+
}
162+
}
163+
return { success: true, output: { success: true, items: [{ a: 1 }], count: 1 } }
164+
})
165+
166+
const result = await apifyRunActorAsyncTool.postProcess!(
167+
startedResult() as any,
168+
{ apiKey: 'k', actorId: 'me/actor' } as any,
169+
executeTool as any
170+
)
171+
172+
expect(fetchSpy).not.toHaveBeenCalled()
173+
expect(executeTool.mock.calls.map(([toolId]) => toolId)).toEqual([
174+
'apify_get_run',
175+
'apify_get_dataset_items',
176+
])
177+
expect(result.output.items).toEqual([{ a: 1 }])
178+
expect(result.output.status).toBe('SUCCEEDED')
179+
})
180+
181+
it('checks the run status before paying the first poll interval', async () => {
182+
const { sleep } = await import('@sim/utils/helpers')
183+
const order: string[] = []
184+
vi.mocked(sleep).mockImplementation(async () => {
185+
order.push('sleep')
186+
})
187+
const executeTool = vi.fn(async () => {
188+
order.push('status')
189+
return {
190+
success: true,
191+
output: { success: true, runId: 'run-1', status: 'SUCCEEDED' },
192+
}
193+
})
194+
195+
await apifyRunActorAsyncTool.postProcess!(
196+
startedResult() as any,
197+
{ apiKey: 'k', actorId: 'me/actor' } as any,
198+
executeTool as any
199+
)
200+
201+
expect(order[0]).toBe('status')
202+
expect(order).not.toContain('sleep')
203+
})
204+
205+
it('reports the real polling window rather than a hardcoded five minutes', async () => {
206+
const executeTool = vi.fn(async () => ({
207+
success: true,
208+
output: { success: true, runId: 'run-1', status: 'RUNNING' },
209+
}))
210+
211+
const result = await apifyRunActorAsyncTool.postProcess!(
212+
startedResult() as any,
213+
{ apiKey: 'k', actorId: 'me/actor' } as any,
214+
executeTool as any
215+
)
216+
217+
expect(result.success).toBe(false)
218+
expect(result.output.status).toBe('TIMEOUT')
219+
expect(result.error).not.toContain('5 minutes')
220+
expect(result.error).toContain(`${DEFAULT_EXECUTION_TIMEOUT_MS / 1000}s`)
221+
})
222+
})

0 commit comments

Comments
 (0)