fix(sessions)!: Never reuse a retired session

docs/guides/code_examples/session_management/sm_standalone.py

@@ -14,12 +14,12 @@ async def main() -> None:
         # Increase the error_score.
         session.mark_bad()
 
-        # Throw away the session.
-        session.retire()
-
         # Lower the error_score and mark the session good.
         session.mark_good()
 
+        # Throw away the session.
+        session.retire()
+
 
 if __name__ == '__main__':
     asyncio.run(main())

docs/upgrading/upgrading_to_v2.md

@@ -272,3 +272,13 @@ When a crawl processes no requests because its request manager holds only handle
 The ERROR message the crawler logs before raising `PersistentRateLimitError` now reads "Giving up the crawl because the request manager can't make progress." instead of "Giving up the crawl because a domain keeps rate-limiting every request."
 
 The two DEBUG messages that `RequestQueue.is_finished` logged, "Background add requests tasks are still in progress." and "The request queue is finished.", are removed together with the method.
+
+## Session retirement is permanent
+
+In v1, `Session.retire()` only added `max_error_score` to the error score, so a later `mark_good()` could lower it again and make the retired session usable. In v2, a separate `retired` flag tracks retirement, exposed as the `Session.retired` property, and a retired session is never usable again. Create a new session to replace it.
+
+On a retired session, `mark_good()` does nothing, so its error score and usage count stay as they are. Calling `retire()` on a session that is already retired also leaves the error score unchanged.
+
+When `mark_good()` or `mark_bad()` retires a session because it expired, reached its usage limit, or reached its error score limit, it only sets the flag. In v1, it also added `max_error_score`. As a result, such sessions have a lower error score in v2, and `is_blocked` is `True` only when the error score itself reached `max_error_score`.
+
+Session state, including the persisted session pool state, has a new `retired` field. Sessions saved by v1 load with `retired=False` and keep their error score.

src/crawlee/sessions/_models.py

@@ -27,6 +27,7 @@ class SessionModel(BaseModel):
     usage_count: int
     max_usage_count: int
     error_score: float
+    retired: bool = False
     cookies: list[CookieParam]
     blocked_status_codes: list[int]
 

src/crawlee/sessions/_session.py

@@ -45,6 +45,7 @@ def __init__(
         usage_count: int = 0,
         max_usage_count: int = 50,
         error_score: float = 0.0,
+        retired: bool = False,
         cookies: SessionCookies | CookieJar | dict[str, str] | list[CookieParam] | None = None,
         blocked_status_codes: list | None = None,
     ) -> None:
@@ -60,6 +61,7 @@ def __init__(
             usage_count: Number of times the session has been used.
             max_usage_count: Maximum allowable uses of the session before it is considered expired.
             error_score: Current error score of the session.
+            retired: Whether the session has been permanently retired.
             cookies: Cookies associated with the session.
             blocked_status_codes: HTTP status codes that indicate a session should be blocked.
         """
@@ -72,6 +74,7 @@ def __init__(
         self._usage_count = usage_count
         self._max_usage_count = max_usage_count
         self._error_score = error_score
+        self._retired = retired
         self._cookies = SessionCookies(cookies) or SessionCookies()
         self._blocked_status_codes = set(blocked_status_codes or self._DEFAULT_BLOCKED_STATUS_CODES)
 
@@ -138,14 +141,19 @@ def usage_count(self) -> float:
         """Get the current usage count."""
         return self._usage_count
 
+    @property
+    def retired(self) -> bool:
+        """Indicate whether the session has been permanently retired."""
+        return self._retired
+
     @property
     def expires_at(self) -> datetime:
         """Get the expiration datetime of the session."""
         return self._created_at + self._max_age
 
     @property
     def is_blocked(self) -> bool:
-        """Indicate whether the session is blocked based on the error score.."""
+        """Indicate whether the session is blocked by its error score."""
         return self._error_score >= self._max_error_score
 
     @property
@@ -161,7 +169,7 @@ def is_max_usage_count_reached(self) -> bool:
     @property
     def is_usable(self) -> bool:
         """Determine if the session is usable for next requests."""
-        return not (self.is_blocked or self.is_expired or self.is_max_usage_count_reached)
+        return not (self._retired or self.is_blocked or self.is_expired or self.is_max_usage_count_reached)
 
     @overload
     def get_state(self, *, as_dict: Literal[True]) -> dict: ...
@@ -183,6 +191,7 @@ def get_state(self, *, as_dict: bool = False) -> SessionModel | dict:
             usage_count=self._usage_count,
             max_usage_count=self._max_usage_count,
             error_score=self._error_score,
+            retired=self._retired,
             cookies=self._cookies.get_cookies_as_dicts(),
             blocked_status_codes=list(self._blocked_status_codes),
         )
@@ -191,15 +200,21 @@ def get_state(self, *, as_dict: bool = False) -> SessionModel | dict:
         return model
 
     def mark_good(self) -> None:
-        """Mark the session as good. Should be called after a successful session usage."""
+        """Mark the session as good. Should be called after a successful session usage.
+
+        A retired session is left unchanged.
+        """
+        if self._retired:
+            return
+
         self._usage_count += 1
 
         if self._error_score > 0:
             self._error_score = max(0, self._error_score - self._error_score_decrement)
 
         # Retire the session if it is not usable anymore
         if not self.is_usable:
-            self.retire()
+            self._retired = True
 
     def mark_bad(self) -> None:
         """Mark the session as bad after an unsuccessful session usage."""
@@ -208,17 +223,19 @@ def mark_bad(self) -> None:
 
         # Retire the session if it is not usable anymore
         if not self.is_usable:
-            self.retire()
+            self._retired = True
 
     def retire(self) -> None:
-        """Retire the session by setting the error score to the maximum value.
+        """Permanently retire the session and add `max_error_score` to its error score, unless it is already retired.
 
         This method should be used if the session usage was unsuccessful and you are sure that it is because of
         the session configuration and not any external matters. For example when server returns 403 status code.
         If the session does not work due to some external factors as server error such as 5XX you probably want
         to use `mark_bad` method.
         """
-        self._error_score += self._max_error_score
+        if not self._retired:
+            self._retired = True
+            self._error_score += self._max_error_score
 
     def is_blocked_status_code(
         self,

tests/unit/sessions/test_session.py

@@ -5,6 +5,7 @@
 import pytest
 
 from crawlee.sessions._cookies import SessionCookies
+from crawlee.sessions._models import SessionModel
 from crawlee.sessions._session import Session
 
 
@@ -35,6 +36,7 @@ def test_session_init(session: Session) -> None:
     assert not session.is_expired
     assert not session.is_max_usage_count_reached
     assert session.is_usable
+    assert not session.retired
 
 
 def test_session_get_state(session: Session) -> None:
@@ -89,6 +91,94 @@ def test_retire_method(session: Session) -> None:
     assert session.usage_count == initial_usage_count
 
 
+def test_mark_good_leaves_retired_session_unchanged() -> None:
+    """A successful request leaves a retired session's state unchanged."""
+    session = Session()
+    session.retire()
+    state = session.get_state(as_dict=True)
+    session.mark_good()
+
+    assert session.get_state(as_dict=True) == state
+    assert session.retired
+    assert session.is_blocked
+    assert not session.is_usable
+
+
+def test_retirement_survives_serialization() -> None:
+    """Retirement survives a JSON round trip even with a reduced error score."""
+    session = Session(retired=True, error_score=1.0)
+    state = session.get_state(as_dict=False).model_dump_json(by_alias=True)
+    restored = Session.from_model(SessionModel.model_validate_json(state))
+    restored.mark_good()
+
+    assert restored.retired
+    assert restored.error_score == 1.0
+    assert restored.usage_count == 0
+    assert not restored.is_blocked
+    assert not restored.is_usable
+
+
+def test_session_without_retirement_field_can_be_restored() -> None:
+    """Saved sessions without a retirement field remain loadable."""
+    state = Session().get_state(as_dict=True)
+    del state['retired']
+    restored = Session.from_model(SessionModel.model_validate(state))
+
+    assert restored.is_usable
+    assert not restored.retired
+
+
+@pytest.mark.parametrize(
+    'mark_method',
+    [pytest.param('mark_good', id='successful-request'), pytest.param('mark_bad', id='failed-request')],
+)
+@pytest.mark.parametrize(
+    'expired',
+    [pytest.param(False, id='usage-limit'), pytest.param(True, id='expired')],
+)
+def test_automatic_retirement_preserves_error_score(mark_method: str, *, expired: bool) -> None:
+    """Expiration and usage limits retire sessions without adding a blocking penalty."""
+    session = Session(
+        error_score=0.5,
+        max_usage_count=10 if expired else 1,
+        created_at=datetime.now(UTC) - timedelta(hours=1) if expired else None,
+    )
+    getattr(session, mark_method)()
+
+    assert session.retired
+    assert not session.is_usable
+    assert not session.is_blocked
+    assert session.error_score == (0.0 if mark_method == 'mark_good' else 1.5)
+    assert session.usage_count == 1
+
+    state = session.get_state(as_dict=True)
+    session.mark_good()
+    session.retire()
+    assert session.get_state(as_dict=True) == state
+
+
+def test_automatic_retirement_at_error_limit_preserves_score() -> None:
+    """Reaching the error limit retires a session without a second error penalty."""
+    session = Session(max_error_score=1.0)
+    session.mark_bad()
+
+    assert session.retired
+    assert session.is_blocked
+    assert not session.is_usable
+    assert session.error_score == 1.0
+    assert session.usage_count == 1
+
+
+def test_retire_is_idempotent() -> None:
+    """Repeated retirement leaves the error score and usage count unchanged."""
+    session = Session()
+    session.retire()
+    state = session.get_state(as_dict=True)
+    session.retire()
+
+    assert session.get_state(as_dict=True) == state
+
+
 def test_mark_good_at_usage_limit_no_double_increment() -> None:
     """Test that mark_good at max usage count does not double-increment usage_count via retire."""
     session = Session(max_usage_count=5, usage_count=4)

tests/unit/sessions/test_session_pool.py

@@ -83,6 +83,19 @@ async def test_get_session(session_pool: SessionPool) -> None:
     assert session.is_usable
 
 
+async def test_pool_does_not_reuse_retired_session_after_success() -> None:
+    """Neither pool lookup returns a retired session after a successful request."""
+    async with SessionPool(max_pool_size=1, persistence_enabled=False) as pool:
+        session = await pool.get_session()
+        session.retire()
+        session.mark_good()
+
+        assert await pool.get_session_by_id(session.id) is None
+        replacement = await pool.get_session()
+        assert replacement.id != session.id
+        assert replacement.is_usable
+
+
 async def test_get_session_no_usable(caplog: pytest.LogCaptureFixture, session_pool: SessionPool) -> None:
     """Ensure that retrieval of a non-existent or retired session returns None and logs warning."""
     session = await session_pool.get_session_by_id('non_existent')
@@ -170,6 +183,40 @@ async def test_session_pool_persist_and_restore(event_manager: EventManager, kvs
         assert previous_state is None
 
 
+async def test_retired_session_is_not_restored(event_manager: EventManager, kvs: KeyValueStore) -> None:
+    """A saved retired session is excluded when the pool is reopened."""
+    service_locator.set_event_manager(event_manager)
+
+    async with SessionPool(
+        max_pool_size=1,
+        persistence_enabled=True,
+        persist_state_kvs_name=KVS_NAME,
+        persist_state_key=PERSIST_STATE_KEY,
+    ) as pool:
+        session = await pool.get_session()
+        session.retire()
+        session.mark_good()
+        event_manager.emit(event=Event.PERSIST_STATE, event_data=EventPersistStateData(is_migrating=False))
+        await event_manager.wait_for_all_listeners_to_complete()
+
+        saved_state = await kvs.get_value(PERSIST_STATE_KEY)
+        assert isinstance(saved_state, dict)
+        saved_pool = SessionPoolModel.model_validate(saved_state)
+        assert saved_pool.sessions[session.id].retired
+        assert not saved_pool.sessions[session.id].is_usable
+
+    async with SessionPool(
+        max_pool_size=1,
+        persistence_enabled=True,
+        persist_state_kvs_name=KVS_NAME,
+        persist_state_key=PERSIST_STATE_KEY,
+    ) as restored_pool:
+        assert await restored_pool.get_session_by_id(session.id) is None
+        replacement = await restored_pool.get_session()
+        assert replacement.id != session.id
+        assert replacement.is_usable
+
+
 async def test_methods_raise_error_when_not_active() -> None:
     session = Session()
     session_pool = SessionPool()