From d4087fb127cf9556c8604bbac3629500d5e4f714 Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Wed, 15 Jul 2026 13:06:06 +0200 Subject: [PATCH 1/4] fix: don't disable Actor exit_process just because scrapy is importable --- docs/02_concepts/01_actor_lifecycle.mdx | 2 +- src/apify/_actor.py | 11 ++++++----- tests/unit/actor/test_actor_lifecycle.py | 24 ++++++++++++++++++++++-- tests/unit/conftest.py | 6 ++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/docs/02_concepts/01_actor_lifecycle.mdx b/docs/02_concepts/01_actor_lifecycle.mdx index 5c1d54351..5e28a0bba 100644 --- a/docs/02_concepts/01_actor_lifecycle.mdx +++ b/docs/02_concepts/01_actor_lifecycle.mdx @@ -48,7 +48,7 @@ You can also create an `Actor` instance dire - `configuration` — a custom `Configuration` instance to control storage paths, API URLs, and other settings. - `configure_logging` — whether to set up default logging configuration (default `True`). Set to `False` if you configure logging yourself. -- `exit_process` — whether the Actor calls `sys.exit()` when the context manager exits. Defaults to `True`, except in IPython, Pytest, and Scrapy environments. +- `exit_process` — whether the Actor calls `sys.exit()` when the context manager exits. Defaults to `True`, except in IPython and Scrapy environments. - `event_listeners_timeout` — maximum time to wait for Actor event listeners to complete before exiting. - `cleanup_timeout` — maximum time to wait for cleanup tasks to finish (default 30 seconds). diff --git a/src/apify/_actor.py b/src/apify/_actor.py index 23c70e9d2..66bb0a14c 100644 --- a/src/apify/_actor.py +++ b/src/apify/_actor.py @@ -1,9 +1,9 @@ from __future__ import annotations import asyncio +import os import sys import warnings -from contextlib import suppress from dataclasses import asdict from datetime import UTC, datetime, timedelta from functools import cached_property @@ -1441,10 +1441,11 @@ def _get_default_exit_process(self) -> bool: self.log.debug('Running in IPython, setting default `exit_process` to False.') return False - # Check if running in Scrapy by attempting to import it. - with suppress(ImportError): - import scrapy # noqa: F401 PLC0415 - + # Detect an actual Scrapy project via the `SCRAPY_SETTINGS_MODULE` environment variable (set by the + # Scrapy CLI and by `apify.scrapy.run_scrapy_actor`), not by whether `scrapy` merely happens to be + # importable. Otherwise any image where Scrapy is a transitive dependency would silently disable the + # `sys.exit()` on Actor exit, so a failed run could end with exit code 0 and be marked as succeeded. + if os.environ.get('SCRAPY_SETTINGS_MODULE'): self.log.debug('Running in Scrapy, setting default `exit_process` to False.') return False diff --git a/tests/unit/actor/test_actor_lifecycle.py b/tests/unit/actor/test_actor_lifecycle.py index d1bcdcbf6..0885c01b4 100644 --- a/tests/unit/actor/test_actor_lifecycle.py +++ b/tests/unit/actor/test_actor_lifecycle.py @@ -18,14 +18,13 @@ from ..._utils import poll_until_condition from apify import Actor +from apify._actor import _ActorType from apify._charging import ChargingManagerImplementation from apify._consts import EXIT_CODE_ERROR_USER_FUNCTION_THREW, ActorEnvVars, ApifyEnvVars if TYPE_CHECKING: from collections.abc import AsyncGenerator, Callable - from apify._actor import _ActorType - @pytest.fixture( params=[ @@ -199,6 +198,27 @@ async def test_unhandled_exception_sets_error_exit_code() -> None: assert actor.exit_code == EXIT_CODE_ERROR_USER_FUNCTION_THREW +# The autouse `_isolate_test_environment` fixture forces the `exit_process` default to False so a clean +# context exit does not call `sys.exit()`. Capture the genuine detector at import time and call it +# directly so these tests exercise the real logic rather than the test-environment override. +_detect_default_exit_process = _ActorType._get_default_exit_process + + +def test_default_exit_process_true_when_scrapy_importable_but_not_running(monkeypatch: pytest.MonkeyPatch) -> None: + """Regression for B7: `scrapy` merely being importable must not disable `exit_process`.""" + pytest.importorskip('scrapy') + monkeypatch.delenv('SCRAPY_SETTINGS_MODULE', raising=False) + actor = Actor(exit_process=False) + assert _detect_default_exit_process(actor) is True + + +def test_default_exit_process_false_when_running_under_scrapy(monkeypatch: pytest.MonkeyPatch) -> None: + """The Scrapy runner sets `SCRAPY_SETTINGS_MODULE`, which must disable `exit_process` by default.""" + monkeypatch.setenv('SCRAPY_SETTINGS_MODULE', 'src.settings') + actor = Actor(exit_process=False) + assert _detect_default_exit_process(actor) is False + + async def test_actor_stops_periodic_events_after_exit(monkeypatch: pytest.MonkeyPatch) -> None: """Test that periodic events (PERSIST_STATE and SYSTEM_INFO) stop emitting after Actor exits.""" monkeypatch.setenv(ApifyEnvVars.SYSTEM_INFO_INTERVAL_MILLIS, '100') diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index c289a3dd9..73f30a21c 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -60,6 +60,12 @@ def prepare_test_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Callabl """ def _prepare_test_env() -> None: + # Production code deliberately does not detect the test environment (see issue #641), so force the + # `exit_process` default to False here. Otherwise a clean Actor context exit would call `sys.exit()` + # and abort the test. Tests that need a specific value pass `exit_process` explicitly. Patch before + # touching the `Actor` proxy below, since that materializes its `_ActorType` instance. + monkeypatch.setattr(apify._actor._ActorType, '_get_default_exit_process', lambda _self: False) + if hasattr(apify._actor.Actor, '__wrapped__'): delattr(apify._actor.Actor, '__wrapped__') From 36f6bd568dead04a8ba0b1e66b6b577e7617748f Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Wed, 15 Jul 2026 15:27:15 +0200 Subject: [PATCH 2/4] refactor: move Scrapy exit_process detection into apify.scrapy._detection --- src/apify/_actor.py | 12 +++++----- src/apify/scrapy/_actor_runner.py | 6 +++++ src/apify/scrapy/_detection.py | 27 +++++++++++++++++++++ tests/unit/actor/test_actor_lifecycle.py | 10 +++++--- tests/unit/scrapy/test_detection.py | 30 ++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 9 deletions(-) create mode 100644 src/apify/scrapy/_detection.py create mode 100644 tests/unit/scrapy/test_detection.py diff --git a/src/apify/_actor.py b/src/apify/_actor.py index 66bb0a14c..21c3310f1 100644 --- a/src/apify/_actor.py +++ b/src/apify/_actor.py @@ -1,7 +1,6 @@ from __future__ import annotations import asyncio -import os import sys import warnings from dataclasses import asdict @@ -1441,11 +1440,12 @@ def _get_default_exit_process(self) -> bool: self.log.debug('Running in IPython, setting default `exit_process` to False.') return False - # Detect an actual Scrapy project via the `SCRAPY_SETTINGS_MODULE` environment variable (set by the - # Scrapy CLI and by `apify.scrapy.run_scrapy_actor`), not by whether `scrapy` merely happens to be - # importable. Otherwise any image where Scrapy is a transitive dependency would silently disable the - # `sys.exit()` on Actor exit, so a failed run could end with exit code 0 and be marked as succeeded. - if os.environ.get('SCRAPY_SETTINGS_MODULE'): + # Delegate the Scrapy check to `apify.scrapy._detection`, but only if it is already imported. A + # non-Scrapy Actor never imports `apify.scrapy` (doing so pulls in Scrapy itself), so this keeps the + # common path from paying that cost, and from disabling `exit_process` just because Scrapy happens to + # be an importable transitive dependency. + scrapy_detection = sys.modules.get('apify.scrapy._detection') + if scrapy_detection is not None and scrapy_detection.is_running_in_scrapy(): self.log.debug('Running in Scrapy, setting default `exit_process` to False.') return False diff --git a/src/apify/scrapy/_actor_runner.py b/src/apify/scrapy/_actor_runner.py index 354b4b323..449883818 100644 --- a/src/apify/scrapy/_actor_runner.py +++ b/src/apify/scrapy/_actor_runner.py @@ -3,6 +3,8 @@ import asyncio from typing import TYPE_CHECKING +from ._detection import mark_running_in_scrapy + if TYPE_CHECKING: from collections.abc import Coroutine @@ -14,6 +16,10 @@ def run_scrapy_actor(coro: Coroutine) -> None: coroutine (typically the Actor's main) by converting it to a Deferred. This bridges the asyncio and Twisted event loops, enabling the Apify and Scrapy integration to work together. """ + # Record that we are driving the process, so the Actor defaults `exit_process` to False (see + # `apify.scrapy._detection`). This runs before the coroutine, hence before the Actor initializes. + mark_running_in_scrapy() + from scrapy.utils.reactor import install_reactor # noqa: PLC0415 from twisted.internet.error import ReactorAlreadyInstalledError # noqa: PLC0415 diff --git a/src/apify/scrapy/_detection.py b/src/apify/scrapy/_detection.py new file mode 100644 index 000000000..f982a2654 --- /dev/null +++ b/src/apify/scrapy/_detection.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +import os + +_running_in_scrapy = False +"""Whether `run_scrapy_actor` is currently driving the process.""" + + +def mark_running_in_scrapy() -> None: + """Record that the current process is being driven by the Apify-Scrapy integration. + + Called by `run_scrapy_actor` before the Actor's main coroutine runs, so `is_running_in_scrapy` already + reports the correct value by the time the Actor initializes. + """ + global _running_in_scrapy # noqa: PLW0603 + _running_in_scrapy = True + + +def is_running_in_scrapy() -> bool: + """Whether the Actor is running as part of a Scrapy project. + + Returns `True` when `run_scrapy_actor` is driving the process, or when the `SCRAPY_SETTINGS_MODULE` + environment variable is set (by the Scrapy CLI or by the Actor's entry point). Detecting a real Scrapy + run this way, rather than by whether `scrapy` merely happens to be importable, keeps the `exit_process` + default correct for images where Scrapy is only a transitive dependency. + """ + return _running_in_scrapy or bool(os.environ.get('SCRAPY_SETTINGS_MODULE')) diff --git a/tests/unit/actor/test_actor_lifecycle.py b/tests/unit/actor/test_actor_lifecycle.py index 0885c01b4..7648c363c 100644 --- a/tests/unit/actor/test_actor_lifecycle.py +++ b/tests/unit/actor/test_actor_lifecycle.py @@ -206,15 +206,19 @@ async def test_unhandled_exception_sets_error_exit_code() -> None: def test_default_exit_process_true_when_scrapy_importable_but_not_running(monkeypatch: pytest.MonkeyPatch) -> None: """Regression for B7: `scrapy` merely being importable must not disable `exit_process`.""" - pytest.importorskip('scrapy') + from apify.scrapy import _detection + + monkeypatch.setattr(_detection, '_running_in_scrapy', False) monkeypatch.delenv('SCRAPY_SETTINGS_MODULE', raising=False) actor = Actor(exit_process=False) assert _detect_default_exit_process(actor) is True def test_default_exit_process_false_when_running_under_scrapy(monkeypatch: pytest.MonkeyPatch) -> None: - """The Scrapy runner sets `SCRAPY_SETTINGS_MODULE`, which must disable `exit_process` by default.""" - monkeypatch.setenv('SCRAPY_SETTINGS_MODULE', 'src.settings') + """A real Scrapy run (the runner flag is set) disables `exit_process` by default.""" + from apify.scrapy import _detection + + monkeypatch.setattr(_detection, '_running_in_scrapy', True) actor = Actor(exit_process=False) assert _detect_default_exit_process(actor) is False diff --git a/tests/unit/scrapy/test_detection.py b/tests/unit/scrapy/test_detection.py new file mode 100644 index 000000000..3d7f8c371 --- /dev/null +++ b/tests/unit/scrapy/test_detection.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from apify.scrapy import _detection + +if TYPE_CHECKING: + import pytest + + +def test_not_in_scrapy_by_default(monkeypatch: pytest.MonkeyPatch) -> None: + """Without the runner flag or `SCRAPY_SETTINGS_MODULE`, the process is not treated as Scrapy.""" + monkeypatch.setattr(_detection, '_running_in_scrapy', False) + monkeypatch.delenv('SCRAPY_SETTINGS_MODULE', raising=False) + assert _detection.is_running_in_scrapy() is False + + +def test_detected_via_settings_module_env_var(monkeypatch: pytest.MonkeyPatch) -> None: + """`SCRAPY_SETTINGS_MODULE` (set by the Scrapy CLI or entry point) marks a Scrapy run.""" + monkeypatch.setattr(_detection, '_running_in_scrapy', False) + monkeypatch.setenv('SCRAPY_SETTINGS_MODULE', 'src.settings') + assert _detection.is_running_in_scrapy() is True + + +def test_detected_via_runner_flag(monkeypatch: pytest.MonkeyPatch) -> None: + """`mark_running_in_scrapy` (called by `run_scrapy_actor`) marks a Scrapy run without any env var.""" + monkeypatch.setattr(_detection, '_running_in_scrapy', False) + monkeypatch.delenv('SCRAPY_SETTINGS_MODULE', raising=False) + _detection.mark_running_in_scrapy() + assert _detection.is_running_in_scrapy() is True From af844341540352a7cf51692f6d18bf43defb2544 Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Wed, 15 Jul 2026 20:38:22 +0200 Subject: [PATCH 3/4] test: force exit_process default to False in integration conftest --- tests/integration/conftest.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c7bd72905..28abd5177 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -56,6 +56,12 @@ def prepare_test_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Callabl """ def _prepare_test_env() -> None: + # Production code deliberately does not detect the test environment (see issue #641), so force the + # `exit_process` default to False here. Otherwise a clean Actor context exit would call `sys.exit()` + # and abort the test. Tests that need a specific value pass `exit_process` explicitly. Patch before + # touching the `Actor` proxy below, since that materializes its `_ActorType` instance. + monkeypatch.setattr(apify._actor._ActorType, '_get_default_exit_process', lambda _self: False) + if hasattr(apify._actor.Actor, '__wrapped__'): delattr(apify._actor.Actor, '__wrapped__') From 06f2feace3af9771d66e9b5e9893db992a99cb37 Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Thu, 16 Jul 2026 12:41:36 +0200 Subject: [PATCH 4/4] chore: shorten exit_process comments and drop internal bug reference --- src/apify/_actor.py | 7 +++---- tests/integration/conftest.py | 7 +++---- tests/unit/actor/test_actor_lifecycle.py | 2 +- tests/unit/conftest.py | 7 +++---- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/apify/_actor.py b/src/apify/_actor.py index 21c3310f1..22af4476b 100644 --- a/src/apify/_actor.py +++ b/src/apify/_actor.py @@ -1440,10 +1440,9 @@ def _get_default_exit_process(self) -> bool: self.log.debug('Running in IPython, setting default `exit_process` to False.') return False - # Delegate the Scrapy check to `apify.scrapy._detection`, but only if it is already imported. A - # non-Scrapy Actor never imports `apify.scrapy` (doing so pulls in Scrapy itself), so this keeps the - # common path from paying that cost, and from disabling `exit_process` just because Scrapy happens to - # be an importable transitive dependency. + # Consult `apify.scrapy._detection` only when it is already imported. A non-Scrapy Actor never + # imports `apify.scrapy` (which pulls in Scrapy itself), so this avoids that cost and the old bug of + # disabling `exit_process` just because Scrapy is an importable transitive dependency. scrapy_detection = sys.modules.get('apify.scrapy._detection') if scrapy_detection is not None and scrapy_detection.is_running_in_scrapy(): self.log.debug('Running in Scrapy, setting default `exit_process` to False.') diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 28abd5177..07f86ea98 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -56,10 +56,9 @@ def prepare_test_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Callabl """ def _prepare_test_env() -> None: - # Production code deliberately does not detect the test environment (see issue #641), so force the - # `exit_process` default to False here. Otherwise a clean Actor context exit would call `sys.exit()` - # and abort the test. Tests that need a specific value pass `exit_process` explicitly. Patch before - # touching the `Actor` proxy below, since that materializes its `_ActorType` instance. + # Production code doesn't detect the test env (#641), so force the `exit_process` default to False. + # Otherwise a clean context exit calls `sys.exit()` and aborts the test. Patch before touching the + # `Actor` proxy below, which materializes its `_ActorType` instance. monkeypatch.setattr(apify._actor._ActorType, '_get_default_exit_process', lambda _self: False) if hasattr(apify._actor.Actor, '__wrapped__'): diff --git a/tests/unit/actor/test_actor_lifecycle.py b/tests/unit/actor/test_actor_lifecycle.py index 7648c363c..a7566c211 100644 --- a/tests/unit/actor/test_actor_lifecycle.py +++ b/tests/unit/actor/test_actor_lifecycle.py @@ -205,7 +205,7 @@ async def test_unhandled_exception_sets_error_exit_code() -> None: def test_default_exit_process_true_when_scrapy_importable_but_not_running(monkeypatch: pytest.MonkeyPatch) -> None: - """Regression for B7: `scrapy` merely being importable must not disable `exit_process`.""" + """`scrapy` merely being importable must not disable `exit_process`.""" from apify.scrapy import _detection monkeypatch.setattr(_detection, '_running_in_scrapy', False) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 73f30a21c..c41eede01 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -60,10 +60,9 @@ def prepare_test_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Callabl """ def _prepare_test_env() -> None: - # Production code deliberately does not detect the test environment (see issue #641), so force the - # `exit_process` default to False here. Otherwise a clean Actor context exit would call `sys.exit()` - # and abort the test. Tests that need a specific value pass `exit_process` explicitly. Patch before - # touching the `Actor` proxy below, since that materializes its `_ActorType` instance. + # Production code doesn't detect the test env (#641), so force the `exit_process` default to False. + # Otherwise a clean context exit calls `sys.exit()` and aborts the test. Patch before touching the + # `Actor` proxy below, which materializes its `_ActorType` instance. monkeypatch.setattr(apify._actor._ActorType, '_get_default_exit_process', lambda _self: False) if hasattr(apify._actor.Actor, '__wrapped__'):