Skip to content

fix: complete SessionError lifecycle and honor reclaim forefront - #2105

Open
Ayush7614 wants to merge 1 commit into
apify:masterfrom
Ayush7614:fix/session-error-lifecycle-and-reclaim-forefront
Open

fix: complete SessionError lifecycle and honor reclaim forefront#2105
Ayush7614 wants to merge 1 commit into
apify:masterfrom
Ayush7614:fix/session-error-lifecycle-and-reclaim-forefront

Conversation

@Ayush7614

@Ayush7614 Ayush7614 commented Jul 30, 2026

Copy link
Copy Markdown

Summary

  • BasicCrawler SessionError path: honor error_handler replacement requests (same as regular errors), wrap handler exceptions in UserDefinedErrorHandlerError, and retire() the blocked session when rotations are exhausted.
  • AdaptivePlaywrightCrawler: re-raise SessionError from the static sub-crawler instead of falling through to the browser with the same blocked session.
  • Retries: reclaim_request now passes forefront=request.forefront so tiered-proxy priority retries actually stay at the front of the queue.

Why

error_handler could replace a request on normal failures but its return value was discarded for SessionError. Exhausted rotations left blocked sessions usable in the pool. Adaptive static SessionError was only logged, so a 403/blocked session could be reused for the browser fallback. 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_retired_when_rotations_exhausted
  • test_reclaim_uses_request_forefront_flag
  • test_static_session_error_propagates_for_rotation
  • 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines 1474 to +1477
if not session:
raise RuntimeError('SessionError raised in a crawling context without a session') from session_error

new_request = None
Comment on lines +1484 to +1488
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants