fix: address PR #35 review findings

CHANGELOG.md

@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 <!-- git-cliff-unreleased-start -->
 ## 0.1.5 - **not yet released**
 
+### ⚠️ Breaking Changes
+
+- `ApifyDatasetLoader.apify_client` is now the private attribute `_apify_client` (a Pydantic `PrivateAttr`). Code that accessed `loader.apify_client` directly will raise `AttributeError`; use the loader's public methods (`load()` / `lazy_load()`) instead of reaching into the client.
+- `APIFY_API_TOKEN` is demoted to a deprecated alias for `APIFY_TOKEN`. Both the `APIFY_API_TOKEN` env var and the `apify_api_token` kwarg still work but now emit a `DeprecationWarning`; migrate to `APIFY_TOKEN` / `apify_token`.
+
 ### 🐛 Bug Fixes
 
 - Update CHANGELOG.md ([#22](https://github.com/apify/langchain-apify/pull/22)) ([eadab67](https://github.com/apify/langchain-apify/commit/eadab67a1d864400d2f72c2fe1532cfa4bd96ddd)) by [@jirispilka](https://github.com/jirispilka)

langchain_apify/_client.py

@@ -8,6 +8,8 @@
 from langchain_apify._constants import (
     _DEFAULT_CRAWLER_TYPE,
     _DEFAULT_DATASET_ITEMS_LIMIT,
+    _DEFAULT_ECOMMERCE_MAX_RESULTS,
+    _DEFAULT_GOOGLE_MAPS_MAX_RESULTS,
     _DEFAULT_GOOGLE_MAX_RESULTS,
     _DEFAULT_LINKEDIN_SEARCH_MAX_RESULTS,
     _DEFAULT_MAX_CRAWL_DEPTH,
@@ -17,6 +19,7 @@
     _DEFAULT_SCRAPE_TIMEOUT_SECS,
     _DEFAULT_SOCIAL_RESULTS_LIMIT,
     _DEFAULT_SOCIAL_TIMEOUT_SECS,
+    _DEFAULT_YOUTUBE_MAX_RESULTS,
 )
 from langchain_apify._error_messages import (
     _ERROR_ACTOR_RUN_FAILED,
@@ -183,7 +186,7 @@ def run_actor_and_get_items(
         if not dataset_id:
             msg = f'Actor {actor_id} run succeeded but returned no default dataset ID.'
             raise RuntimeError(msg)
-        items = self._list_items_or_raise(dataset_id, dataset_items_limit)
+        items = self.get_dataset_items(dataset_id, dataset_items_limit)
         return run, items
 
     def run_task(
@@ -252,10 +255,10 @@ def run_task_and_get_items(
         if not dataset_id:
             msg = f'Task {task_id} run succeeded but returned no default dataset ID.'
             raise RuntimeError(msg)
-        items = self._list_items_or_raise(dataset_id, dataset_items_limit)
+        items = self.get_dataset_items(dataset_id, dataset_items_limit)
         return run, items
 
-    def _scrape_url(
+    def scrape_url_with_metadata(
         self, url: str, timeout_secs: int = _DEFAULT_SCRAPE_TIMEOUT_SECS
     ) -> tuple[dict, list[dict], str, str]:
         """Scrape a single URL and return run/items/content metadata.
@@ -297,10 +300,10 @@ def _scrape_url(
     def scrape_url(self, url: str, timeout_secs: int = _DEFAULT_SCRAPE_TIMEOUT_SECS) -> str:
         """Scrape a single URL and return only the page content.
 
-        Thin public wrapper over :meth:`_scrape_url` for callers that don't need
-        the run/items metadata.
+        Thin public wrapper over :meth:`scrape_url_with_metadata` for callers
+        that don't need the run/items metadata.
         """
-        _, _, content, _ = self._scrape_url(url=url, timeout_secs=timeout_secs)
+        _, _, content, _ = self.scrape_url_with_metadata(url=url, timeout_secs=timeout_secs)
         return content
 
     def instagram_scrape(
@@ -504,7 +507,7 @@ def linkedin_profile_search(
     def google_maps_search(
         self,
         query: str,
-        max_results: int = 10,
+        max_results: int = _DEFAULT_GOOGLE_MAPS_MAX_RESULTS,
         language: str | None = None,
         timeout_secs: int = _DEFAULT_RUN_TIMEOUT_SECS,
     ) -> tuple[dict, list[dict]]:
@@ -726,7 +729,7 @@ def youtube_scrape(
         self,
         search_query: str,
         search_type: str = 'search',
-        max_results: int = 10,
+        max_results: int = _DEFAULT_YOUTUBE_MAX_RESULTS,
         timeout_secs: int = _DEFAULT_RUN_TIMEOUT_SECS,
     ) -> tuple[dict, list[dict]]:
         """Scrape YouTube videos, channels, or search results.
@@ -768,7 +771,7 @@ def ecommerce_scrape(
         self,
         url: str,
         url_type: str = 'product',
-        max_results: int = 20,
+        max_results: int = _DEFAULT_ECOMMERCE_MAX_RESULTS,
         timeout_secs: int = _DEFAULT_RUN_TIMEOUT_SECS,
     ) -> tuple[dict, list[dict]]:
         """Extract product data from an e-commerce URL.
@@ -848,14 +851,6 @@ def crawl_website(
         )
         return items
 
-    def _list_items_or_raise(self, dataset_id: str, limit: int) -> list[dict]:
-        """Fetch dataset items, wrapping any network error in a RuntimeError."""
-        try:
-            return self._client.dataset(dataset_id).list_items(limit=limit, clean=True).items
-        except _TRANSPORT_EXCEPTIONS as exc:
-            msg = f'Apify dataset fetch failed for {dataset_id}: {exc}'
-            raise RuntimeError(msg) from exc
-
     @staticmethod
     def _check_run_status(run: dict) -> None:
         """Raise if the run did not succeed."""

langchain_apify/_utils.py

@@ -13,6 +13,9 @@
 _MAX_DESCRIPTION_LEN: int = 350
 
 _DEPRECATED_APIFY_API_TOKEN_MSG = "The 'apify_api_token' parameter is deprecated, use 'apify_token' instead."
+_DEPRECATED_APIFY_API_TOKEN_ENV_MSG = (
+    "The 'APIFY_API_TOKEN' environment variable is deprecated, use 'APIFY_TOKEN' instead."
+)
 _BOTH_TOKENS_MSG = (
     "Both 'apify_token' and 'apify_api_token' were specified; using 'apify_token' "
     "and ignoring the deprecated 'apify_api_token'."
@@ -61,10 +64,15 @@ def _resolve_apify_token() -> str | None:
     """Resolve the Apify API token from environment variables.
 
     ``APIFY_TOKEN`` (SDK-standard) takes precedence; ``APIFY_API_TOKEN`` is
-    kept as a fallback for backwards compatibility with this package's
-    historical naming.
+    kept as a deprecated fallback for backwards compatibility with this
+    package's historical naming, and emits a ``DeprecationWarning`` when used.
     """
-    return os.getenv('APIFY_TOKEN') or os.getenv('APIFY_API_TOKEN')
+    if token := os.getenv('APIFY_TOKEN'):
+        return token
+    if token := os.getenv('APIFY_API_TOKEN'):
+        warnings.warn(_DEPRECATED_APIFY_API_TOKEN_ENV_MSG, DeprecationWarning, stacklevel=2)
+        return token
+    return None
 
 
 def _apify_token_secret_factory() -> SecretStr | None:

langchain_apify/retrievers.py

@@ -11,6 +11,7 @@
 
 from langchain_apify._client import ApifyToolsClient
 from langchain_apify._constants import _DEFAULT_RAG_MAX_RESULTS, _DEFAULT_RUN_TIMEOUT_SECS
+from langchain_apify._error_messages import _ERROR_APIFY_TOKEN_ENV_VAR_NOT_SET
 from langchain_apify._utils import (
     _apify_token_secret_factory,
     _extract_content,
@@ -75,10 +76,14 @@ def _handle_deprecated_apify_api_token(cls, values: dict) -> dict:
     def model_post_init(self, context: Any) -> None:  # noqa: ANN401
         """Construct the underlying ``ApifyToolsClient``.
 
-        The helper handles ``None`` / ``SecretStr`` / env-fallback and raises
-        ``ValueError`` if no token is available.
+        Mirrors ``_ApifyGenericTool``: guard against a missing token locally
+        before constructing the client, so the failure mode is consistent
+        across tools and the retriever.
         """
-        self._client = ApifyToolsClient(apify_token=self.apify_token)
+        if self.apify_token is None:
+            msg = _ERROR_APIFY_TOKEN_ENV_VAR_NOT_SET
+            raise ValueError(msg)
+        self._client = ApifyToolsClient(apify_token=self.apify_token.get_secret_value())
         super().model_post_init(context)
 
     def _get_relevant_documents(

langchain_apify/tools/__init__.py

@@ -7,17 +7,7 @@
 from __future__ import annotations
 
 from langchain_apify.tools.actors import ApifyActorsTool
-from langchain_apify.tools.base import (
-    _TOOL_RUN_ERRORS,
-    _VALID_MEMORY_MBYTES,
-    _ApifyGenericTool,
-    _iso,
-    _run_meta,
-)
 from langchain_apify.tools.core import (
-    _DESC_DATASET_ITEMS_LIMIT,
-    _DESC_MEMORY_MBYTES,
-    _DESC_RUN_TIMEOUT_SECS,
     APIFY_CORE_TOOLS,
     ApifyGetDatasetItemsInput,
     ApifyGetDatasetItemsTool,
@@ -116,12 +106,4 @@
     'TikTokSearchType',
     'TwitterSearchMode',
     'TwitterSort',
-    '_DESC_DATASET_ITEMS_LIMIT',
-    '_DESC_MEMORY_MBYTES',
-    '_DESC_RUN_TIMEOUT_SECS',
-    '_TOOL_RUN_ERRORS',
-    '_VALID_MEMORY_MBYTES',
-    '_ApifyGenericTool',
-    '_iso',
-    '_run_meta',
 ]

langchain_apify/tools/core.py

@@ -19,7 +19,7 @@
     _MAX_MEMORY_MBYTES_CAP,
     _MAX_TIMEOUT_SECS_CAP,
 )
-from langchain_apify.tools.base import _ApifyGenericTool
+from langchain_apify.tools.base import _TOOL_RUN_ERRORS, _ApifyGenericTool
 
 if TYPE_CHECKING:
     from langchain_core.callbacks import CallbackManagerForToolRun
@@ -163,7 +163,7 @@ def _run(
             run = self._client.run_actor(
                 actor_id, run_input, self._clamp_timeout(timeout_secs), self._clamp_memory(memory_mbytes)
             )
-        except RuntimeError as exc:
+        except _TOOL_RUN_ERRORS as exc:
             raise ToolException(str(exc)) from exc
         return self._envelope(run, [])
 
@@ -213,7 +213,7 @@ def _run(
     ) -> str:
         try:
             items = self._client.get_dataset_items(dataset_id, self._clamp_items(limit), max(0, offset))
-        except RuntimeError as exc:
+        except _TOOL_RUN_ERRORS as exc:
             raise ToolException(str(exc)) from exc
         return self._envelope(None, items)
 
@@ -277,7 +277,7 @@ def _run(
                 self._clamp_memory(memory_mbytes),
                 self._clamp_items(dataset_items_limit),
             )
-        except RuntimeError as exc:
+        except _TOOL_RUN_ERRORS as exc:
             raise ToolException(str(exc)) from exc
         return self._envelope(run, items)
 
@@ -326,10 +326,10 @@ def _run(
         _run_manager: CallbackManagerForToolRun | None = None,
     ) -> str:
         try:
-            # _scrape_url is the rich primitive; scrape_url() drops the metadata
-            # this tool needs (run + content source), so access it directly.
-            run, _, content, _ = self._client._scrape_url(url, self._clamp_timeout(timeout_secs))  # noqa: SLF001
-        except RuntimeError as exc:
+            # scrape_url_with_metadata is the rich primitive; the plain
+            # scrape_url() drops the run metadata + content source this tool needs.
+            run, _, content, _ = self._client.scrape_url_with_metadata(url, self._clamp_timeout(timeout_secs))
+        except _TOOL_RUN_ERRORS as exc:
             raise ToolException(str(exc)) from exc
         return self._envelope(run, [{'url': url, 'content': content}])
 
@@ -389,7 +389,7 @@ def _run(
             run = self._client.run_task(
                 task_id, task_input, self._clamp_timeout(timeout_secs), self._clamp_memory(memory_mbytes)
             )
-        except RuntimeError as exc:
+        except _TOOL_RUN_ERRORS as exc:
             raise ToolException(str(exc)) from exc
         return self._envelope(run, [])
 
@@ -453,7 +453,7 @@ def _run(
                 self._clamp_memory(memory_mbytes),
                 self._clamp_items(dataset_items_limit),
             )
-        except RuntimeError as exc:
+        except _TOOL_RUN_ERRORS as exc:
             raise ToolException(str(exc)) from exc
         return self._envelope(run, items)
 

tests/unit_tests/test_actor_tools.py

@@ -24,7 +24,7 @@
     ApifyYouTubeScraperTool,
 )
 from langchain_apify._client import ApifyToolsClient
-from langchain_apify.tools import _ApifyGenericTool
+from langchain_apify.tools.base import _ApifyGenericTool
 from tests.unit_tests.conftest import SAMPLE_ITEMS, SUCCEEDED_RUN, make_tool
 
 # ---------------------------------------------------------------------------

tests/unit_tests/test_client.py

@@ -640,30 +640,30 @@ def test_build_instagram_url_post_from_id() -> None:
 
 
 # ---------------------------------------------------------------------------
-# _scrape_url
+# scrape_url_with_metadata
 # ---------------------------------------------------------------------------
 
 
-def test__scrape_url_returns_markdown_and_metadata(client: ApifyToolsClient, mock_apify_client: MagicMock) -> None:
+def test_scrape_url_with_metadata_returns_markdown(client: ApifyToolsClient, mock_apify_client: MagicMock) -> None:
     mock_apify_client.actor.return_value.call.return_value = SUCCEEDED_RUN
     mock_apify_client.dataset.return_value.list_items.return_value.items = [
         {'markdown': '# Hello', 'text': 'Hello', 'url': 'https://example.com'},
     ]
 
-    run, items, content, source = client._scrape_url('https://example.com')
+    run, items, content, source = client.scrape_url_with_metadata('https://example.com')
     assert run == SUCCEEDED_RUN
     assert items
     assert content == '# Hello'
     assert source == 'markdown'
 
 
-def test__scrape_url_falls_back_to_text(client: ApifyToolsClient, mock_apify_client: MagicMock) -> None:
+def test_scrape_url_with_metadata_falls_back_to_text(client: ApifyToolsClient, mock_apify_client: MagicMock) -> None:
     mock_apify_client.actor.return_value.call.return_value = SUCCEEDED_RUN
     mock_apify_client.dataset.return_value.list_items.return_value.items = [
         {'text': 'Plain text content', 'url': 'https://example.com'},
     ]
 
-    _, _, content, source = client._scrape_url('https://example.com')
+    _, _, content, source = client.scrape_url_with_metadata('https://example.com')
     assert content == 'Plain text content'
     assert source == 'text'
 

tests/unit_tests/test_deprecated_token_alias.py

@@ -18,6 +18,7 @@
 
 import warnings
 from contextlib import ExitStack
+from typing import TYPE_CHECKING
 from unittest.mock import MagicMock, patch
 
 from apify_client._types import ListPage
@@ -27,8 +28,12 @@
 
 from langchain_apify import ApifyDatasetLoader, ApifyWrapper
 from langchain_apify._client import ApifyToolsClient
+from langchain_apify._utils import _resolve_apify_token
 from langchain_apify.tools import ApifyActorsTool, ApifyRunActorTool
 
+if TYPE_CHECKING:
+    import pytest
+
 # ---------------------------------------------------------------------------
 # Helpers
 # ---------------------------------------------------------------------------
@@ -263,3 +268,55 @@ def test_both_specified_uses_apify_token(self) -> None:
             assert 'ignoring' in str(w[0].message)
             assert tool.apify_token is not None
             assert tool.apify_token.get_secret_value() == 'primary'
+
+
+# ---------------------------------------------------------------------------
+# APIFY_API_TOKEN environment variable (deprecated alias for APIFY_TOKEN)
+# ---------------------------------------------------------------------------
+
+
+class TestApifyApiTokenEnvVarAlias:
+    """The ``APIFY_API_TOKEN`` env var is honoured but deprecated.
+
+    Mirrors the kwarg matrix: ``APIFY_TOKEN`` is preferred and silent,
+    ``APIFY_API_TOKEN`` works but emits a ``DeprecationWarning``.
+    """
+
+    def test_apify_token_env_no_warning(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        monkeypatch.setenv('APIFY_TOKEN', 'new-style')
+        monkeypatch.delenv('APIFY_API_TOKEN', raising=False)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            token = _resolve_apify_token()
+        assert token == 'new-style'
+        assert len(w) == 0
+
+    def test_apify_api_token_env_emits_warning(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        monkeypatch.delenv('APIFY_TOKEN', raising=False)
+        monkeypatch.setenv('APIFY_API_TOKEN', 'legacy-style')
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            token = _resolve_apify_token()
+        assert token == 'legacy-style'
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+        assert 'APIFY_API_TOKEN' in str(w[0].message)
+
+    def test_apify_token_env_takes_precedence_silently(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """When both env vars are set, ``APIFY_TOKEN`` wins with no warning."""
+        monkeypatch.setenv('APIFY_TOKEN', 'primary')
+        monkeypatch.setenv('APIFY_API_TOKEN', 'ignored')
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            token = _resolve_apify_token()
+        assert token == 'primary'
+        assert len(w) == 0
+
+    def test_no_token_env_returns_none_without_warning(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        monkeypatch.delenv('APIFY_TOKEN', raising=False)
+        monkeypatch.delenv('APIFY_API_TOKEN', raising=False)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            token = _resolve_apify_token()
+        assert token is None
+        assert len(w) == 0