fix: preserve partial action results when mid-batch action fails#4770
Conversation
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/agent/service.py">
<violation number="1" location="browser_use/agent/service.py:2810">
P1: `multi_act()` now swallows `InterruptedError` from `_check_stop_or_pause()`, so pause/stop control flow no longer propagates to callers and can be treated as a normal action failure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
|
Great fix! Preserving partial multi_act() results instead of losing them is the right call. The cubic bot raises a valid concern: InterruptedError from _check_stop_or_pause() will now be swallowed as a regular ActionResult. Suggest explicitly re-raising it: except Exception as e:
if isinstance(e, (InterruptedError, asyncio.CancelledError)):
raise # Preserve stop/pause control flow
results.append(ActionResult(error=str(e)))
return resultsOtherwise the core fix is solid - minimal, well-scoped, and the downstream flows already handle the returned error result correctly. |
|
Good catch, thanks! Added the |
|
@cubic-dev-ai the |
@toller892 You're right—the current code in commit The latest diff for # Re-raise control-flow exceptions to preserve stop/pause/interrupt behavior
if isinstance(e, (InterruptedError, asyncio.CancelledError)):
raiseThis correctly preserves the agent's stop and pause behavior while still achieving the goal of returning partial For more details on how I process updates and manage review history, you can check the Cubic documentation. |
When multi_act() executes a batch of actions and one fails mid-way, the partial results from successfully executed earlier actions were discarded by re-raising the exception. The agent lost visibility into which actions completed before the failure. Return the accumulated results with an error ActionResult appended, which aligns with the existing post_process logic that explicitly handles multi-action errors via loop detection and replan nudges (lines 1221-1222).
PHclaw pointed out that _check_stop_or_pause() raises InterruptedError inside the multi_act loop. Converting it to an ActionResult would prevent the agent from properly stopping/pausing between batched actions. Re-raise InterruptedError and asyncio.CancelledError before wrapping other exceptions as ActionResult with partial results.
d117d30 to
fd7eeeb
Compare
- Re-indent the except block to match the inner try (was a SyntaxError) - Drop asyncio.CancelledError from the isinstance check (it's BaseException, never reaches except Exception) - Re-raise on _is_connection_like_error so _handle_step_error can run its reconnect / browser-closed shutdown logic - Include exception class name in the preserved ActionResult error
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/agent/service.py">
<violation number="1" location="browser_use/agent/service.py:2815">
P2: Re-raising connection-like errors here discards already-succeeded actions in the same batch, because the caller only records `multi_act()` results on normal return and the step error handler replaces them with a single generic error result.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Problem
When
multi_act()executes a batch of N actions and action K (where 0 < K < N) raises an exception, the partial results from successfully executed actions 1..K-1 are silently lost. The exception propagates through_execute_actions()→_handle_step_error(), which creates a single genericActionResult(error=error_msg)that overwrites any trace of the earlier successes.The LLM receives only the generic error and loses visibility into which specific actions completed before the failure — even though those actions had real side effects (clicks, form fills, navigation).
Fix
Instead of re-raising the exception, append an error
ActionResultto the accumulatedresultslist and return it. This path is already supported downstream:_execute_actions()(line 1205):self.state.last_resultis set from the return value_post_process()(lines 1221-1222): explicitly handles multi-action errors via loop detection and replan nudges_finalize(): saves all results to history for the LLM to seeChange:
raise e→results.append(ActionResult(error=str(e))); return resultsImpact
consecutive_failures, multi-action errors are handled by loop detectionSummary by cubic
Preserves partial results in
multi_act()when a mid-batch action fails by returning accumulated successes plus a trailing errorActionResult. Keeps stop/pause and reconnect/shutdown behavior by re-raisingInterruptedErrorand connection-like errors.ActionResult(error="<Exception>: <msg>"), and return accumulated results.InterruptedError;asyncio.CancelledErrordoesn’t reach this handler; re-raise connection-like errors so_handle_step_errorcan run reconnect/shutdown.exceptblock indentation to match the innertry(prevented a SyntaxError).Written for commit 54322e6. Summary will update on new commits.