fix: honor error_handler replacement requests on SessionError and preserve forefront on retries

[Ayush7614]

Summary

• BasicCrawler SessionError path: honor error_handler replacement requests only while session rotations remain; treat same unique_key as no replacement (avoids marking the in-progress request handled while add_request is a no-op); wrap handler exceptions in UserDefinedErrorHandlerError and set RequestState.ERROR when the handler raises or rotations are exhausted.
• Shared _handle_error_handler_replacement helper for the SessionError and normal retry paths: inherits retry_count / session_rotation_count, passes forefront= to add_request, records record_request_processing_finish on the replaced original, and retires blocked sessions before awaits on the SessionError path.
• Retries: reclaim_request 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. A field-wise new_request != request check treated same-URL replacements as new work and could drop the original. 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_same_unique_key_rotates
• test_session_error_handler_replacement_ignored_when_rotations_exhausted
• test_session_error_handler_exception_is_wrapped
• test_reclaim_uses_request_forefront_flag
• Existing session rotation / error_handler tests still pass

[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.

[Mantisus]

Hey, @Ayush7614. Thank you for your contribution!

[Ayush7614]

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!

[Ayush7614]

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

[Ayush7614]

Thanks @vdusek

Addressed in a138011:

• Treat new_request.unique_key == request.unique_key as no replacement (same for the normal retry path via a shared _handle_error_handler_replacement helper).
• Helper inherits retry_count / session_rotation_count, uses add_request(..., forefront=), calls record_request_processing_finish on the replaced original, sets RequestState.ERROR when the handler raises, and retires the session before awaits on the SessionError path.
• Added test_session_error_handler_same_unique_key_rotates and test_session_error_handler_exception_is_wrapped.
• Simplified the forefront reclaim assertion and dropped unnecessary queue.drop() calls.

[vdusek]

LGTM