fix(crawlers): Keep the crawler usable after a failed run (#2255) · apify/crawlee-python@3e73166 · GitHub
Skip to content

Commit 3e73166

Browse files
authored
fix(crawlers): Keep the crawler usable after a failed run (#2255)
Follow-up to #2229, split out of #2239. ## Crawler state after a failed run - An exception during setup (e.g. `add_requests`) left `_running` set, so every later `run()` raised `RuntimeError`. The whole run body is now inside the `try`/`finally`. - After an earlier successful run, a failed run's implicit purge dropped the requests it left pending. A `_last_run_failed` flag now skips that purge, even with `purge_request_queue=True`. A failure in the post-crawl steps (statistics, `_save_crawler_state()`) counts as a failed run. ## AutoscaledPool - `run.result.exception()` is guarded against a cancelled future. On SIGINT it raised `CancelledError` in the orchestrator's `finally`, and the error log was lost. - The orchestrator's fallback error message now reads `Unpropagated exception in worker task orchestrator`, so it can be told apart from the same message in `run()`. *✍️ Drafted by Claude Code*
1 parent c6fba87 commit 3e73166

4 files changed

Lines changed: 197 additions & 68 deletions

File tree

‎src/crawlee/_autoscaling/autoscaled_pool.py‎

Lines changed: 2 additions & 2 deletions

‎src/crawlee/crawlers/_basic/_basic_crawler.py‎

Lines changed: 78 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ async def persist_state_factory() -> KeyValueStore:
507507
self._keep_alive = keep_alive
508508
self._running = False
509509
self._has_finished_before = False
510+
self._last_run_failed = False
510511
self._failed = False
511512
self._unexpected_stop = False
512513
self._logger_once = LoggerOnce(self._logger)
@@ -696,8 +697,8 @@ async def run(
696697
requests: The requests to be enqueued before the crawler starts.
697698
purge_request_queue: If this is `True` and the crawler is not being run for the first time, the request
698699
queue will be purged. A run that ended with an exception does not count as a previous run, so a
699-
retry keeps the requests that were still pending. Named request queues are considered persistent
700-
and are never purged implicitly.
700+
retry keeps the requests that were still pending even when this is `True`. Named request queues
701+
are considered persistent and are never purged implicitly.
701702
"""
702703
if self._running:
703704
raise RuntimeError(
@@ -706,86 +707,97 @@ async def run(
706707

707708
self._running = True
708709

709-
if self._respect_robots_txt_file and not isinstance(self._request_manager, ThrottlingRequestManager):
710-
self._logger.warning(
711-
'The `respect_robots_txt_file` option is enabled, but the crawler is not using '
712-
'`ThrottlingRequestManager`. Crawl-delay directives from robots.txt will not be enforced. To enable '
713-
'crawl-delay support, configure the crawler to use `ThrottlingRequestManager` as the request manager.'
714-
)
715-
716-
if self._has_finished_before:
717-
await self._statistics.reset()
718-
719-
if self._use_session_pool:
720-
await self._session_pool.reset_store()
721-
722-
if purge_request_queue:
723-
request_manager = await self.get_request_manager()
724-
# A `ThrottlingRequestManager` delegates `purge` to the manager it wraps, so inspect the wrapped
725-
# manager when deciding whether the purge would hit a named queue.
726-
inner_manager = (
727-
request_manager.inner if isinstance(request_manager, ThrottlingRequestManager) else request_manager
710+
try:
711+
if self._respect_robots_txt_file and not isinstance(self._request_manager, ThrottlingRequestManager):
712+
self._logger.warning(
713+
'The `respect_robots_txt_file` option is enabled, but the crawler is not using '
714+
'`ThrottlingRequestManager`. Crawl-delay directives from robots.txt will not be enforced. To '
715+
'enable crawl-delay support, configure the crawler to use `ThrottlingRequestManager` as the '
716+
'request manager.'
728717
)
729-
# Named storages are persistent and shared across runs, so they are never purged implicitly
730-
# (the same named-storage exemption as in `StorageClient._purge_if_needed`).
731-
is_named_queue = isinstance(inner_manager, RequestQueue) and inner_manager.name is not None
732-
if not is_named_queue:
733-
await request_manager.purge()
734718

735-
if requests is not None:
736-
await self.add_requests(requests)
737-
738-
interrupted = False
719+
if self._has_finished_before:
720+
await self._statistics.reset()
721+
722+
if self._use_session_pool:
723+
await self._session_pool.reset_store()
724+
725+
# A failed run does not count as a previous run, so its pending requests survive into the retry.
726+
if purge_request_queue and not self._last_run_failed:
727+
request_manager = await self.get_request_manager()
728+
# A `ThrottlingRequestManager` delegates `purge` to the manager it wraps, so inspect the wrapped
729+
# manager when deciding whether the purge would hit a named queue.
730+
inner_manager = (
731+
request_manager.inner
732+
if isinstance(request_manager, ThrottlingRequestManager)
733+
else request_manager
734+
)
735+
# Named storages are persistent and shared across runs, so they are never purged implicitly
736+
# (the same named-storage exemption as in `StorageClient._purge_if_needed`).
737+
is_named_queue = isinstance(inner_manager, RequestQueue) and inner_manager.name is not None
738+
if not is_named_queue:
739+
await request_manager.purge()
739740

740-
def sigint_handler() -> None:
741-
nonlocal interrupted
741+
if requests is not None:
742+
await self.add_requests(requests)
742743

743-
if not interrupted:
744-
interrupted = True
745-
self._logger.info('Pausing... Press CTRL+C again to force exit.')
744+
interrupted = False
746745

747-
run_task.cancel()
746+
def sigint_handler() -> None:
747+
nonlocal interrupted
748748

749-
run_task = asyncio.create_task(self._run_crawler(), name='run_crawler_task')
749+
if not interrupted:
750+
interrupted = True
751+
self._logger.info('Pausing... Press CTRL+C again to force exit.')
750752

751-
if threading.current_thread() is threading.main_thread(): # `add_signal_handler` works only in the main thread
752-
with suppress(NotImplementedError): # event loop signal handlers are not supported on Windows
753-
asyncio.get_running_loop().add_signal_handler(signal.SIGINT, sigint_handler)
753+
run_task.cancel()
754754

755-
try:
756-
await run_task
757-
except CancelledError:
758-
pass
759-
finally:
760-
# A failed run must leave the instance usable, so that the caller can retry after handling the error.
761-
self._running = False
755+
run_task = asyncio.create_task(self._run_crawler(), name='run_crawler_task')
762756

757+
# `add_signal_handler` works only in the main thread
763758
if threading.current_thread() is threading.main_thread():
764-
with suppress(NotImplementedError):
765-
asyncio.get_running_loop().remove_signal_handler(signal.SIGINT)
759+
with suppress(NotImplementedError): # event loop signal handlers are not supported on Windows
760+
asyncio.get_running_loop().add_signal_handler(signal.SIGINT, sigint_handler)
766761

767-
if self._statistics.error_tracker.total > 0:
768-
self._logger.info(
769-
'Error analysis:'
770-
f' total_errors={self._statistics.error_tracker.total}'
771-
f' unique_errors={self._statistics.error_tracker.unique_error_count}'
772-
)
762+
try:
763+
await run_task
764+
except CancelledError:
765+
pass
766+
finally:
767+
if threading.current_thread() is threading.main_thread():
768+
with suppress(NotImplementedError):
769+
asyncio.get_running_loop().remove_signal_handler(signal.SIGINT)
770+
771+
if self._statistics.error_tracker.total > 0:
772+
self._logger.info(
773+
'Error analysis:'
774+
f' total_errors={self._statistics.error_tracker.total}'
775+
f' unique_errors={self._statistics.error_tracker.unique_error_count}'
776+
)
773777

774-
if interrupted:
775-
self._logger.info(
776-
f'The crawl was interrupted. To resume, do: CRAWLEE_PURGE_ON_START=0 python {sys.argv[0]}'
777-
)
778+
if interrupted:
779+
self._logger.info(
780+
f'The crawl was interrupted. To resume, do: CRAWLEE_PURGE_ON_START=0 python {sys.argv[0]}'
781+
)
778782

779-
self._has_finished_before = True
783+
self._has_finished_before = True
784+
self._last_run_failed = False
780785

781-
await self._save_crawler_state()
786+
await self._save_crawler_state()
782787

783-
final_statistics = self._statistics.calculate()
784-
if self._statistics_log_format == 'table':
785-
self._logger.info(f'Final request statistics:\n{final_statistics.to_table()}')
788+
final_statistics = self._statistics.calculate()
789+
if self._statistics_log_format == 'table':
790+
self._logger.info(f'Final request statistics:\n{final_statistics.to_table()}')
791+
else:
792+
self._logger.info('Final request statistics:', extra=final_statistics.to_dict())
793+
except BaseException:
794+
self._last_run_failed = True
795+
raise
786796
else:
787-
self._logger.info('Final request statistics:', extra=final_statistics.to_dict())
788-
return final_statistics
797+
return final_statistics
798+
finally:
799+
# A failed run must leave the instance usable, so that the caller can retry after handling the error.
800+
self._running = False
789801

790802
async def _run_crawler(self) -> None:
791803
local_event_manager = self._service_locator.get_event_manager()

‎tests/unit/_autoscaling/test_autoscaled_pool.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
import asyncio
6+
import logging
67
from contextlib import suppress
78
from datetime import timedelta
89
from itertools import chain, repeat
@@ -164,6 +165,31 @@ async def is_finished() -> bool:
164165
await asyncio.gather(pool_run_task, return_exceptions=True)
165166

166167

168+
async def test_orchestrator_error_is_logged_when_the_run_is_cancelled(
169+
system_status: SystemStatus | Mock, caplog: pytest.LogCaptureFixture
170+
) -> None:
171+
"""A scheduling error raised while the run is being cancelled is logged, as the cancelled result cannot carry it."""
172+
173+
async def is_finished() -> bool:
174+
pool_run_task.cancel()
175+
raise RuntimeError('Queue status unavailable')
176+
177+
pool = AutoscaledPool(
178+
system_status=system_status,
179+
run_task_function=lambda: future(None),
180+
is_task_ready_function=lambda: future(False),
181+
is_finished_function=is_finished,
182+
)
183+
184+
with caplog.at_level(logging.ERROR, logger='crawlee._autoscaling.autoscaled_pool'):
185+
pool_run_task = asyncio.create_task(pool.run())
186+
with pytest.raises(asyncio.CancelledError):
187+
await pool_run_task
188+
189+
assert 'Unpropagated exception in worker task orchestrator' in caplog.text
190+
assert 'Queue status unavailable' in caplog.text
191+
192+
167193
async def test_propagates_exceptions_after_finished(system_status: SystemStatus | Mock) -> None:
168194
started_count = 0
169195

‎tests/unit/crawlers/_basic/test_basic_crawler.py‎

Lines changed: 91 additions & 0 deletions

0 commit comments

Comments
 (0)