Skip to content

Commit d4087fb

Browse files
committed
fix: don't disable Actor exit_process just because scrapy is importable
1 parent 785da90 commit d4087fb

4 files changed

Lines changed: 35 additions & 8 deletions

File tree

docs/02_concepts/01_actor_lifecycle.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ You can also create an <ApiLink to="class/Actor">`Actor`</ApiLink> instance dire
4848

4949
- `configuration` — a custom <ApiLink to="class/Configuration">`Configuration`</ApiLink> instance to control storage paths, API URLs, and other settings.
5050
- `configure_logging` — whether to set up default logging configuration (default `True`). Set to `False` if you configure logging yourself.
51-
- `exit_process` — whether the Actor calls `sys.exit()` when the context manager exits. Defaults to `True`, except in IPython, Pytest, and Scrapy environments.
51+
- `exit_process` — whether the Actor calls `sys.exit()` when the context manager exits. Defaults to `True`, except in IPython and Scrapy environments.
5252
- `event_listeners_timeout` — maximum time to wait for Actor event listeners to complete before exiting.
5353
- `cleanup_timeout` — maximum time to wait for cleanup tasks to finish (default 30 seconds).
5454

src/apify/_actor.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
from __future__ import annotations
22

33
import asyncio
4+
import os
45
import sys
56
import warnings
6-
from contextlib import suppress
77
from dataclasses import asdict
88
from datetime import UTC, datetime, timedelta
99
from functools import cached_property
@@ -1441,10 +1441,11 @@ def _get_default_exit_process(self) -> bool:
14411441
self.log.debug('Running in IPython, setting default `exit_process` to False.')
14421442
return False
14431443

1444-
# Check if running in Scrapy by attempting to import it.
1445-
with suppress(ImportError):
1446-
import scrapy # noqa: F401 PLC0415
1447-
1444+
# Detect an actual Scrapy project via the `SCRAPY_SETTINGS_MODULE` environment variable (set by the
1445+
# Scrapy CLI and by `apify.scrapy.run_scrapy_actor`), not by whether `scrapy` merely happens to be
1446+
# importable. Otherwise any image where Scrapy is a transitive dependency would silently disable the
1447+
# `sys.exit()` on Actor exit, so a failed run could end with exit code 0 and be marked as succeeded.
1448+
if os.environ.get('SCRAPY_SETTINGS_MODULE'):
14481449
self.log.debug('Running in Scrapy, setting default `exit_process` to False.')
14491450
return False
14501451

tests/unit/actor/test_actor_lifecycle.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@
1818

1919
from ..._utils import poll_until_condition
2020
from apify import Actor
21+
from apify._actor import _ActorType
2122
from apify._charging import ChargingManagerImplementation
2223
from apify._consts import EXIT_CODE_ERROR_USER_FUNCTION_THREW, ActorEnvVars, ApifyEnvVars
2324

2425
if TYPE_CHECKING:
2526
from collections.abc import AsyncGenerator, Callable
2627

27-
from apify._actor import _ActorType
28-
2928

3029
@pytest.fixture(
3130
params=[
@@ -199,6 +198,27 @@ async def test_unhandled_exception_sets_error_exit_code() -> None:
199198
assert actor.exit_code == EXIT_CODE_ERROR_USER_FUNCTION_THREW
200199

201200

201+
# The autouse `_isolate_test_environment` fixture forces the `exit_process` default to False so a clean
202+
# context exit does not call `sys.exit()`. Capture the genuine detector at import time and call it
203+
# directly so these tests exercise the real logic rather than the test-environment override.
204+
_detect_default_exit_process = _ActorType._get_default_exit_process
205+
206+
207+
def test_default_exit_process_true_when_scrapy_importable_but_not_running(monkeypatch: pytest.MonkeyPatch) -> None:
208+
"""Regression for B7: `scrapy` merely being importable must not disable `exit_process`."""
209+
pytest.importorskip('scrapy')
210+
monkeypatch.delenv('SCRAPY_SETTINGS_MODULE', raising=False)
211+
actor = Actor(exit_process=False)
212+
assert _detect_default_exit_process(actor) is True
213+
214+
215+
def test_default_exit_process_false_when_running_under_scrapy(monkeypatch: pytest.MonkeyPatch) -> None:
216+
"""The Scrapy runner sets `SCRAPY_SETTINGS_MODULE`, which must disable `exit_process` by default."""
217+
monkeypatch.setenv('SCRAPY_SETTINGS_MODULE', 'src.settings')
218+
actor = Actor(exit_process=False)
219+
assert _detect_default_exit_process(actor) is False
220+
221+
202222
async def test_actor_stops_periodic_events_after_exit(monkeypatch: pytest.MonkeyPatch) -> None:
203223
"""Test that periodic events (PERSIST_STATE and SYSTEM_INFO) stop emitting after Actor exits."""
204224
monkeypatch.setenv(ApifyEnvVars.SYSTEM_INFO_INTERVAL_MILLIS, '100')

tests/unit/conftest.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ def prepare_test_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Callabl
6060
"""
6161

6262
def _prepare_test_env() -> None:
63+
# Production code deliberately does not detect the test environment (see issue #641), so force the
64+
# `exit_process` default to False here. Otherwise a clean Actor context exit would call `sys.exit()`
65+
# and abort the test. Tests that need a specific value pass `exit_process` explicitly. Patch before
66+
# touching the `Actor` proxy below, since that materializes its `_ActorType` instance.
67+
monkeypatch.setattr(apify._actor._ActorType, '_get_default_exit_process', lambda _self: False)
68+
6369
if hasattr(apify._actor.Actor, '__wrapped__'):
6470
delattr(apify._actor.Actor, '__wrapped__')
6571

0 commit comments

Comments
 (0)