fix: complete SessionError lifecycle and honor reclaim forefront by Ayush7614 · Pull Request #2105 · apify/crawlee-python

Ayush7614 · 2026-07-30T14:20:18Z

Summary

BasicCrawler SessionError path: honor error_handler replacement requests only while session rotations remain; wrap handler exceptions in UserDefinedErrorHandlerError; set RequestState.ERROR when rotations are exhausted.
Retries: reclaim_request now passes forefront=request.forefront so tiered-proxy priority retries stay at the front of the queue.
session.retire() stays on the retry/rotation path only (preserves long-lived sessions when max_session_rotations=0).

Why

error_handler could replace a request on normal failures, but its return value was discarded for SessionError. Tiered proxies set request.forefront = True on retry, but reclaim always used the default forefront=False.

Test plan

test_session_error_handler_can_replace_request
test_session_error_handler_replacement_ignored_when_rotations_exhausted
test_reclaim_uses_request_forefront_flag
Existing session rotation / error_handler tests still pass

Honor error_handler replacement requests for SessionError, retire blocked sessions when rotations are exhausted, propagate AdaptivePlaywright static SessionError for rotation instead of browser fallback, and reclaim retries with request.forefront for tiered-proxy priority.

Copilot

🟡 Not ready to approve

The SessionError path currently leaves requests in an inconsistent lifecycle state and misses retry/error tracking in one replacement branch, which can lead to incorrect persisted request metadata and statistics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR completes the SessionError lifecycle in BasicCrawler/AdaptivePlaywrightCrawler and ensures retry reclaiming respects request queue priority (forefront) so tiered-proxy retries stay at the front of the queue.

Changes:

Honor error_handler return values for SessionError, wrap handler exceptions consistently, and retire sessions when rotations are exhausted.
Re-raise SessionError from the adaptive crawler’s static path to trigger session rotation instead of falling through to the browser.
Pass forefront=request.forefront into reclaim_request so priority retries preserve queue ordering.

File summaries

File	Description
`src/crawlee/crawlers/_basic/_basic_crawler.py`	Updates retry reclaim behavior to honor `forefront`, and refines `SessionError` handling (rotation/retire + error_handler honoring).
`src/crawlee/crawlers/_adaptive_playwright/_adaptive_playwright_crawler.py`	Ensures static-path `SessionError` propagates to enable session rotation rather than browser fallback with the same session.
`tests/unit/crawlers/_basic/test_basic_crawler.py`	Adds unit tests covering `SessionError` error_handler replacement, session retirement on exhausted rotations, and `forefront` reclaim behavior.
`tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py`	Adds a unit test asserting static `SessionError` propagation triggers session rotation and prevents browser fallback.

Review details

Suppressed comments (1)

src/crawlee/crawlers/_basic/_basic_crawler.py:1504

When session rotations are exhausted, the request is marked as handled without setting its final state to ERROR. This leaves failed requests in REQUEST_HANDLER state in storage, which diverges from the normal error path (where the request is set to RequestState.ERROR before marking handled).

            else:
                # Exhausted rotations: retire the blocked session so it is not reused from the pool.
                session.retire()
                await self._mark_request_as_handled(request)

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

            if not session:
                raise RuntimeError('SessionError raised in a crawling context without a session') from session_error

+            new_request = None


+            if new_request is not None and new_request != request:
+                await request_manager.add_request(new_request)
+                await self._mark_request_as_handled(request)
+                session.retire()
+                return


Mantisus

Hey, @Ayush7614. Thank you for your contribution!

Mantisus · 2026-08-02T18:53:12Z

+                    if isinstance(static_run.exception, SessionError):
+                        raise static_run.exception


The old behavior is correct here. A site can block the HTTP client even when the same session still works in a browser, for example, by requiring JavaScript execution before granting access.

Reverted — static SessionError again falls through to the browser path as before.

Mantisus · 2026-08-02T19:03:51Z

+                session.retire()
+                return


Replacement with new_request should only apply in the retry branch. Please drop the early return here because it also triggers when session rotations are exhausted, so the request gets replaced instead of failed and failed_request_handler never runs.

Fixed — replacement is only applied inside the _should_retry_request branch.

Mantisus · 2026-08-02T19:04:52Z

+                # Exhausted rotations: retire the blocked session so it is not reused from the pool.
+                session.retire()


Please keep session.retire() only in the _should_retry_request branch. Otherwise, it breaks the long-lived session use case when max_session_rotations=0.

Fixed — session.retire() is only called on the retry/rotation path now.

Mantisus · 2026-08-02T19:40:54Z

+async def test_reclaim_uses_request_forefront_flag() -> None:
+    """Retries must reclaim with `request.forefront` so tiered-proxy priority retries stay at the front."""
+    queue = await RequestQueue.open()
+    reclaim_calls: list[bool] = []
+    original_reclaim = queue.reclaim_request
+
+    async def tracking_reclaim(request: Request, *, forefront: bool = False) -> Any:
+        reclaim_calls.append(forefront)
+        return await original_reclaim(request, forefront=forefront)
+
+    queue.reclaim_request = tracking_reclaim  # type: ignore[method-assign]
+
+    crawler = BasicCrawler(request_manager=queue, max_request_retries=1)
+
+    @crawler.router.default_handler
+    async def handler(context: BasicCrawlingContext) -> None:
+        context.request.forefront = True
+        raise RuntimeError('retry me')
+
+    await crawler.run([Request.from_url('https://a.placeholder.com')])
+
+    assert reclaim_calls == [True]
+
+    await queue.drop()


Let's use the mock

Suggested change

async def test_reclaim_uses_request_forefront_flag() -> None:

"""Retries must reclaim with `request.forefront` so tiered-proxy priority retries stay at the front."""

queue = await RequestQueue.open()

reclaim_calls: list[bool] = []

original_reclaim = queue.reclaim_request

async def tracking_reclaim(request: Request, *, forefront: bool = False) -> Any:

reclaim_calls.append(forefront)

return await original_reclaim(request, forefront=forefront)

queue.reclaim_request = tracking_reclaim # type: ignore[method-assign]

crawler = BasicCrawler(request_manager=queue, max_request_retries=1)

@crawler.router.default_handler

async def handler(context: BasicCrawlingContext) -> None:

context.request.forefront = True

raise RuntimeError('retry me')

await crawler.run([Request.from_url('https://a.placeholder.com')])

assert reclaim_calls == [True]

await queue.drop()

async def test_reclaim_uses_request_forefront_flag() -> None:

"""Retries must reclaim with `request.forefront` so tiered-proxy priority retries stay at the front."""

queue = await RequestQueue.open()

crawler = BasicCrawler(request_manager=queue, max_request_retries=1)

@crawler.router.default_handler

async def handler(context: BasicCrawlingContext) -> None:

context.request.forefront = True

raise RuntimeError('Arbitrary crash for testing purposes')

with patch.object(queue, 'reclaim_request', wraps=queue.reclaim_request) as reclaim_mock:

await crawler.run(['https://a.placeholder.com'])

reclaim_mock.assert_awaited_once()

(reclaimed_request,), reclaim_kwargs = reclaim_mock.await_args_list[0]

assert reclaimed_request.url == 'https://a.placeholder.com'

assert reclaim_kwargs == {'forefront': True}

await queue.drop()

Updated the test to use patch.object(queue, 'reclaim_request', wraps=...) as suggested.

Honor error_handler replacements only while rotations remain, keep session.retire() on the retry path only, restore AdaptivePlaywright static SessionError fallback to browser, and use patch.object for forefront reclaim coverage.

Ayush7614 · 2026-08-03T13:35:28Z

Thanks for the review @Mantisus!

Addressed in 3a44e62:

Reverted the AdaptivePlaywright static SessionError re-raise — browser fallback with the same session is intentional when HTTP is blocked but JS may still work.
error_handler replacement requests are honored only inside the _should_retry_request branch, so exhausted rotations still hit failed_request_handler.
session.retire() is only called on the retry/rotation path (preserves max_session_rotations=0 long-lived sessions).
Exhausted path now sets RequestState.ERROR before marking handled.
Forefront reclaim test updated to use patch.object(..., wraps=...).

Mantisus

One nit, and please update the PR description to match the current state. Otherwise LGTM, thanks for contributing!

Mantisus · 2026-08-03T17:48:18Z

+
+    @crawler.error_handler
+    async def error_handler(context: BasicCrawlingContext, error: Exception) -> Request | None:
+        assert isinstance(error, SessionError)


This assert is caught by the crawler and re-raised as UserDefinedErrorHandlerError.

Assertions raised inside error_handler are wrapped as UserDefinedErrorHandlerError, so remove the isinstance check.

Ayush7614 · 2026-08-03T19:54:19Z

Thanks @Mantisus — addressed in 20ccb2b:

removed the assert isinstance(error, SessionError) from the error_handler test
updated the PR description to match the current scope

vdusek requested review from Mantisus and Copilot July 31, 2026 09:03

Copilot started reviewing on behalf of vdusek July 31, 2026 09:04 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Mantisus suggested changes Aug 2, 2026

View reviewed changes

fix: address SessionError review feedback

3a44e62

Honor error_handler replacements only while rotations remain, keep session.retire() on the retry path only, restore AdaptivePlaywright static SessionError fallback to browser, and use patch.object for forefront reclaim coverage.

Mantisus approved these changes Aug 3, 2026

View reviewed changes

test: drop assert from SessionError error_handler test

20ccb2b

Assertions raised inside error_handler are wrapped as UserDefinedErrorHandlerError, so remove the isinstance check.

Mantisus requested a review from vdusek August 3, 2026 20:07

		if isinstance(static_run.exception, SessionError):
		raise static_run.exception

		# Exhausted rotations: retire the blocked session so it is not reused from the pool.
		session.retire()

Uh oh!

Conversation

Ayush7614 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Not ready to approve

Suppressed comments (1)

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ayush7614 commented Jul 30, 2026 •

edited

Loading