Skip to content

Commit 870d7ef

Browse files
committed
fix(tools): close operation lifecycle gaps
1 parent 2526ff8 commit 870d7ef

4 files changed

Lines changed: 111 additions & 19 deletions

File tree

apps/sim/lib/internal/browser-use/operations/run-task.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,54 @@ describe('executeRunTaskOperation', () => {
233233
)
234234
expect(mockFetch.mock.calls[2]?.[1]?.signal).not.toBe(controller.signal)
235235
})
236+
237+
it('stops an automatically created task session when polling is cancelled', async () => {
238+
const controller = new AbortController()
239+
const abortError = new DOMException('cancelled', 'AbortError')
240+
mockFetch
241+
.mockResolvedValueOnce(jsonResponse({ id: 'task-1', sessionId: 'task-session' }))
242+
.mockImplementationOnce(async () => {
243+
controller.abort(abortError)
244+
throw abortError
245+
})
246+
.mockResolvedValueOnce(new Response(null, { status: 204 }))
247+
248+
await expect(
249+
executeRunTaskOperation({ task: 'Open the page', apiKey: 'api-key' }, controller.signal)
250+
).rejects.toBe(abortError)
251+
252+
expect(mockFetch).toHaveBeenNthCalledWith(
253+
3,
254+
'https://api.browser-use.com/api/v2/sessions/task-session',
255+
expect.objectContaining({
256+
method: 'PATCH',
257+
body: JSON.stringify({ action: 'stop' }),
258+
signal: expect.any(AbortSignal),
259+
})
260+
)
261+
})
262+
263+
it('stops an automatically created task session when polling times out', async () => {
264+
const now = vi.spyOn(Date, 'now').mockReturnValueOnce(0).mockReturnValue(1_000_000_000_000_000)
265+
mockFetch
266+
.mockResolvedValueOnce(jsonResponse({ id: 'task-1', sessionId: 'task-session' }))
267+
.mockResolvedValueOnce(jsonResponse({ status: 'running', sessionId: 'task-session' }))
268+
.mockResolvedValueOnce(
269+
jsonResponse({ shareUrl: 'https://browser-use.com/share/task-session' })
270+
)
271+
.mockResolvedValueOnce(new Response(null, { status: 204 }))
272+
273+
const result = await executeRunTaskOperation({ task: 'Open the page', apiKey: 'api-key' })
274+
now.mockRestore()
275+
276+
expect(result).toMatchObject({
277+
success: false,
278+
error: expect.stringContaining('Task did not complete within the maximum polling time'),
279+
})
280+
expect(mockFetch).toHaveBeenNthCalledWith(
281+
4,
282+
'https://api.browser-use.com/api/v2/sessions/task-session',
283+
expect.objectContaining({ method: 'PATCH', signal: expect.any(AbortSignal) })
284+
)
285+
})
236286
})

apps/sim/lib/internal/browser-use/operations/run-task.ts

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ async function fetchTaskStatus(
304304

305305
interface PollResult {
306306
success: boolean
307+
taskEnded: boolean
307308
output: unknown
308309
steps: BrowserUseTaskStep[]
309310
sessionId: string | null
@@ -312,12 +313,18 @@ interface PollResult {
312313
error?: string
313314
}
314315

316+
interface PollOptions {
317+
initialSessionId: string | null
318+
signal?: AbortSignal
319+
onSessionId: (sessionId: string) => void
320+
}
321+
315322
async function pollForCompletion(
316323
taskId: string,
317324
apiKey: string,
318-
initialSessionId: string | null,
319-
signal?: AbortSignal
325+
options: PollOptions
320326
): Promise<PollResult> {
327+
const { initialSessionId, signal, onSessionId } = options
321328
let consecutiveErrors = 0
322329
let sessionId = initialSessionId
323330
let liveUrl: string | null = null
@@ -337,6 +344,7 @@ async function pollForCompletion(
337344
if (consecutiveErrors >= MAX_CONSECUTIVE_ERRORS) {
338345
return {
339346
success: false,
347+
taskEnded: false,
340348
output: null,
341349
steps: [],
342350
sessionId,
@@ -352,7 +360,10 @@ async function pollForCompletion(
352360

353361
consecutiveErrors = 0
354362
const taskData = result.data
355-
if (taskData.sessionId) sessionId = taskData.sessionId
363+
if (taskData.sessionId) {
364+
sessionId = taskData.sessionId
365+
onSessionId(taskData.sessionId)
366+
}
356367
const status = taskData.status
357368

358369
logger.info(`BrowserUse task ${taskId} status: ${status}`)
@@ -370,6 +381,7 @@ async function pollForCompletion(
370381
const output = taskData.output ?? null
371382
return {
372383
success: status === 'finished',
384+
taskEnded: true,
373385
output,
374386
steps: taskData.steps ?? [],
375387
sessionId,
@@ -391,11 +403,14 @@ async function pollForCompletion(
391403
if (finalResult.ok && ['finished', 'failed', 'stopped'].includes(finalResult.data.status)) {
392404
const status = finalResult.data.status
393405
const output = finalResult.data.output ?? null
406+
const finalSessionId = finalResult.data.sessionId ?? sessionId
407+
if (finalSessionId) onSessionId(finalSessionId)
394408
return {
395409
success: status === 'finished',
410+
taskEnded: true,
396411
output,
397412
steps: finalResult.data.steps ?? [],
398-
sessionId: finalResult.data.sessionId ?? sessionId,
413+
sessionId: finalSessionId,
399414
liveUrl,
400415
publicShareUrl,
401416
error:
@@ -409,6 +424,7 @@ async function pollForCompletion(
409424

410425
return {
411426
success: false,
427+
taskEnded: false,
412428
output: null,
413429
steps: [],
414430
sessionId,
@@ -470,20 +486,22 @@ export const executeRunTaskOperation: InternalToolOperationImplementation<
470486
params: BrowserUseRunTaskParams,
471487
signal?: AbortSignal
472488
): Promise<BrowserUseRunTaskResponse> => {
473-
let sessionId: string | undefined
489+
let profileSessionId: string | undefined
490+
let taskSessionId: string | null = null
491+
let taskEnded = false
474492

475493
if (params.profile_id) {
476494
logger.info(`Creating session with profile ID: ${params.profile_id}`)
477495
const sessionResult = await createSessionWithProfile(params.profile_id, params.apiKey, signal)
478496
if ('error' in sessionResult) {
479497
return { success: false, output: emptyOutput(), error: sessionResult.error }
480498
}
481-
sessionId = sessionResult.sessionId
499+
profileSessionId = sessionResult.sessionId
482500
}
483501

484502
try {
485-
const requestBody = buildRequestBody(params, sessionId)
486-
logger.info('Creating BrowserUse task', { hasSession: !!sessionId })
503+
const requestBody = buildRequestBody(params, profileSessionId)
504+
logger.info('Creating BrowserUse task', { hasSession: !!profileSessionId })
487505
const response = await fetchBrowserUse('/tasks', params.apiKey, {
488506
method: 'POST',
489507
body: requestBody,
@@ -512,10 +530,18 @@ export const executeRunTaskOperation: InternalToolOperationImplementation<
512530
}
513531
const data = parsed.data
514532
const taskId = data.id
515-
const initialSessionId = sessionId ?? data.sessionId ?? null
533+
const initialSessionId = profileSessionId ?? data.sessionId ?? null
534+
taskSessionId = initialSessionId
516535
logger.info(`Created BrowserUse task ${taskId}`, { sessionId: initialSessionId })
517536

518-
const result = await pollForCompletion(taskId, params.apiKey, initialSessionId, signal)
537+
const result = await pollForCompletion(taskId, params.apiKey, {
538+
initialSessionId,
539+
signal,
540+
onSessionId: (discoveredSessionId) => {
541+
taskSessionId = discoveredSessionId
542+
},
543+
})
544+
taskEnded = result.taskEnded
519545

520546
const finalSessionId = result.sessionId ?? initialSessionId
521547
const shareUrl =
@@ -544,7 +570,10 @@ export const executeRunTaskOperation: InternalToolOperationImplementation<
544570
error: `Error creating task: ${getErrorMessage(error, 'Unknown error')}`,
545571
}
546572
} finally {
547-
if (sessionId) {
573+
const sessionsToStop = new Set<string>()
574+
if (profileSessionId) sessionsToStop.add(profileSessionId)
575+
if (!taskEnded && taskSessionId) sessionsToStop.add(taskSessionId)
576+
for (const sessionId of sessionsToStop) {
548577
await stopSession(sessionId, params.apiKey)
549578
}
550579
}

apps/sim/lib/internal/supabase/operations/storage-update-bucket.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,18 @@ describe('executeStorageUpdateBucketOperation', () => {
9090
executeStorageUpdateBucketOperation(INPUT, controller.signal)
9191
).rejects.toMatchObject({ name: 'AbortError' })
9292
})
93+
94+
it('returns a structured failure for an invalid project reference', async () => {
95+
const result = await executeStorageUpdateBucketOperation({
96+
...INPUT,
97+
projectId: '../invalid',
98+
})
99+
100+
expect(result).toMatchObject({
101+
success: false,
102+
output: { message: 'Failed to update storage bucket', results: {} },
103+
error: expect.any(String),
104+
})
105+
expect(fetchMock).not.toHaveBeenCalled()
106+
})
93107
})

apps/sim/lib/internal/supabase/operations/storage-update-bucket.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@ export const executeStorageUpdateBucketOperation: InternalToolOperationImplement
1212
params: SupabaseStorageUpdateBucketParams,
1313
signal
1414
): Promise<SupabaseStorageUpdateBucketResponse> => {
15-
const baseUrl = supabaseBaseUrl(params.projectId)
16-
const bucket = encodeStorageSegment(params.bucket)
17-
const headers = {
18-
apikey: params.apiKey,
19-
Authorization: `Bearer ${params.apiKey}`,
20-
'Content-Type': 'application/json',
21-
}
22-
2315
try {
16+
const baseUrl = supabaseBaseUrl(params.projectId)
17+
const bucket = encodeStorageSegment(params.bucket)
18+
const headers = {
19+
apikey: params.apiKey,
20+
Authorization: `Bearer ${params.apiKey}`,
21+
'Content-Type': 'application/json',
22+
}
2423
const currentResponse = await fetch(`${baseUrl}/storage/v1/bucket/${bucket}`, {
2524
method: 'GET',
2625
headers,

0 commit comments

Comments
 (0)