Skip to content

Commit cb7822e

Browse files
fix: propagate autoscaled pool scheduling errors (#2229)
### Description When a request manager's `is_empty()` or `is_finished()` raises, `BasicCrawler.run()` currently logs the orchestrator error but returns normally with final statistics. A failed queue status check can therefore look like a successfully completed crawl while requests remain pending. Retain scheduling exceptions until the existing worker cleanup finishes, then fail the pool's result future instead of resolving it successfully. Preserve a result already set by a worker failure or abort. The regression tests exercise the crawler with a real filesystem queue, inject errors only at its status methods, and verify that a new crawler can process the pending request after recovery. A separate event-controlled test checks that an active worker finishes before the scheduling error reaches the caller. ### Issues Found during source review; no existing issue linked. This concerns scheduling callbacks, distinct from the worker-task timeout handling in #2009. ### Testing - Five new regression cases fail on unmodified `0853d79e670cba27f9692919688305d1cc565c6c` because no exception reaches the caller. - Python 3.10.20: all five regression cases pass. - Python 3.13.14: `pytest tests/unit/_autoscaling tests/unit/crawlers/_basic tests/unit/_utils/test_recurring_task.py -k 'not test_send_request_works'` passes 157 tests, with 1 skipped and 2 deselected. - Full Ruff lint and formatting checks pass (599 files); full `ty check --python-platform linux` and changed-file native Windows typing pass. - Native Windows full typing reports two `BaseContext.Process` diagnostics in unchanged `test_system.py`; the same diagnostics occur on the unmodified base. The two existing `test_send_request_works` cases fail with JSON decoding errors against the local test server on both the patch and the unmodified base; they are excluded from the final related-suite run. The full repository/browser/service matrix was not run locally. --------- Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
1 parent 6203496 commit cb7822e

4 files changed

Lines changed: 98 additions & 7 deletions

File tree

src/crawlee/_autoscaling/autoscaled_pool.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ class AutoscaledPool:
4040
"""Manages a pool of asynchronous resource-intensive tasks that are executed in parallel.
4141
4242
The pool keeps `min_concurrency` tasks running even while the system is overloaded, and starts additional tasks
43-
only if there is enough free CPU and memory available. If an exception is thrown in any of the tasks, it is
44-
propagated and the pool is stopped.
43+
only if there is enough free CPU and memory available. If an exception is thrown in any of the tasks or in the
44+
pool's scheduling loop, it is propagated and the pool is stopped.
4545
"""
4646

4747
_AUTOSCALE_INTERVAL = timedelta(seconds=10)
@@ -106,7 +106,7 @@ def __init__(
106106
async def run(self) -> None:
107107
"""Start the autoscaled pool and return when all tasks are completed and `is_finished_function` returns True.
108108
109-
If there is an exception in one of the tasks, it will be re-raised.
109+
If a task or the pool's scheduling loop raises an exception, it will be re-raised.
110110
"""
111111
if self._current_run is not None:
112112
raise RuntimeError('The pool is already running')
@@ -217,6 +217,7 @@ async def _worker_task_orchestrator(self, run: _AutoscaledPoolRun) -> None:
217217
Exits when `is_finished_function` returns True.
218218
"""
219219
finished = False
220+
orchestrator_error: Exception | None = None
220221

221222
try:
222223
while not (finished := await self._is_finished_function()) and not run.result.done():
@@ -244,6 +245,10 @@ async def _worker_task_orchestrator(self, run: _AutoscaledPoolRun) -> None:
244245

245246
with suppress(asyncio.TimeoutError):
246247
await asyncio.wait_for(run.worker_tasks_updated.wait(), timeout=0.5)
248+
except Exception as exc:
249+
# Surface the error through `run.result` only once the cleanup below has awaited the worker tasks,
250+
# so that the caller does not observe the failure while tasks are still in flight.
251+
orchestrator_error = exc
247252
finally:
248253
if finished:
249254
logger.debug('`is_finished_function` reports that we are finished')
@@ -258,7 +263,13 @@ async def _worker_task_orchestrator(self, run: _AutoscaledPoolRun) -> None:
258263
logger.debug('Terminating - no running tasks to wait for')
259264

260265
if not run.result.done():
261-
run.result.set_result(object())
266+
if orchestrator_error is not None:
267+
run.result.set_exception(orchestrator_error)
268+
else:
269+
run.result.set_result(object())
270+
elif orchestrator_error is not None:
271+
# A worker failure or an abort already decided the run, so this error has no way out.
272+
logger.error('Exception in worker task orchestrator', exc_info=orchestrator_error)
262273

263274
def _reap_worker_task(self, task: asyncio.Task, run: _AutoscaledPoolRun) -> None:
264275
"""Handle cleanup and tracking of a completed worker task.

src/crawlee/crawlers/_basic/_basic_crawler.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -695,8 +695,9 @@ async def run(
695695
Args:
696696
requests: The requests to be enqueued before the crawler starts.
697697
purge_request_queue: If this is `True` and the crawler is not being run for the first time, the request
698-
queue will be purged. Named request queues are considered persistent and are never purged
699-
implicitly.
698+
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.
700701
"""
701702
if self._running:
702703
raise RuntimeError(
@@ -756,6 +757,9 @@ def sigint_handler() -> None:
756757
except CancelledError:
757758
pass
758759
finally:
760+
# A failed run must leave the instance usable, so that the caller can retry after handling the error.
761+
self._running = False
762+
759763
if threading.current_thread() is threading.main_thread():
760764
with suppress(NotImplementedError):
761765
asyncio.get_running_loop().remove_signal_handler(signal.SIGINT)
@@ -772,7 +776,6 @@ def sigint_handler() -> None:
772776
f'The crawl was interrupted. To resume, do: CRAWLEE_PURGE_ON_START=0 python {sys.argv[0]}'
773777
)
774778

775-
self._running = False
776779
self._has_finished_before = True
777780

778781
await self._save_crawler_state()

tests/unit/_autoscaling/test_autoscaled_pool.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,47 @@ async def run() -> None:
123123
assert done_count < 20
124124

125125

126+
async def test_orchestrator_error_waits_for_running_worker(system_status: SystemStatus | Mock) -> None:
127+
"""A scheduling error reaches the caller only after the worker tasks that were still running have finished."""
128+
worker_started = asyncio.Event()
129+
worker_finished = asyncio.Event()
130+
check_failed = asyncio.Event()
131+
release_worker = asyncio.Event()
132+
checks = 0
133+
134+
async def run() -> None:
135+
worker_started.set()
136+
await release_worker.wait()
137+
worker_finished.set()
138+
139+
async def is_finished() -> bool:
140+
nonlocal checks
141+
checks += 1
142+
if checks > 1:
143+
await worker_started.wait()
144+
check_failed.set()
145+
raise RuntimeError('Queue status unavailable')
146+
return False
147+
148+
pool = AutoscaledPool(
149+
system_status=system_status,
150+
run_task_function=run,
151+
is_task_ready_function=lambda: future(True),
152+
is_finished_function=is_finished,
153+
)
154+
pool_run_task = asyncio.create_task(pool.run())
155+
try:
156+
await asyncio.wait_for(check_failed.wait(), timeout=5)
157+
assert not pool_run_task.done()
158+
release_worker.set()
159+
with pytest.raises(RuntimeError, match='Queue status unavailable'):
160+
await asyncio.wait_for(pool_run_task, timeout=5)
161+
assert worker_finished.is_set()
162+
finally:
163+
release_worker.set()
164+
await asyncio.gather(pool_run_task, return_exceptions=True)
165+
166+
126167
async def test_propagates_exceptions_after_finished(system_status: SystemStatus | Mock) -> None:
127168
started_count = 0
128169

tests/unit/crawlers/_basic/test_basic_crawler.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,42 @@ async def handler(context: BasicCrawlingContext) -> None:
6161
assert calls == ['https://a.placeholder.com', 'https://b.placeholder.com', 'https://c.placeholder.com']
6262

6363

64+
@pytest.mark.parametrize(
65+
'method',
66+
[pytest.param('is_empty', id='is_empty'), pytest.param('is_finished', id='is_finished')],
67+
)
68+
@pytest.mark.parametrize(
69+
'error_type',
70+
[pytest.param(RuntimeError, id='runtime error'), pytest.param(asyncio.TimeoutError, id='timeout error')],
71+
)
72+
async def test_propagates_request_queue_status_errors(
73+
monkeypatch: pytest.MonkeyPatch, method: str, error_type: type[Exception]
74+
) -> None:
75+
"""A request manager failure reaches the caller, and the crawler can be run again once the manager recovers."""
76+
queue = await RequestQueue.open()
77+
await queue.add_request('https://a.placeholder.com')
78+
crawler = BasicCrawler(request_manager=queue)
79+
handled_urls = []
80+
81+
@crawler.router.default_handler
82+
async def handler(context: BasicCrawlingContext) -> None:
83+
handled_urls.append(context.request.url)
84+
85+
error = error_type('Queue status unavailable')
86+
with monkeypatch.context() as monkey:
87+
monkey.setattr(queue, method, AsyncMock(side_effect=error))
88+
with pytest.raises(error_type, match='Queue status unavailable') as exc_info:
89+
await crawler.run()
90+
assert exc_info.value is error
91+
92+
assert handled_urls == []
93+
assert not await queue.is_finished()
94+
95+
await crawler.run()
96+
assert handled_urls == ['https://a.placeholder.com']
97+
assert await queue.is_finished()
98+
99+
64100
async def test_processes_requests_from_request_source_tandem() -> None:
65101
request_queue = await RequestQueue.open()
66102
await request_queue.add_requests(

0 commit comments

Comments
 (0)