From 8724fb2b8e41ab544dae692e388d0275915853f7 Mon Sep 17 00:00:00 2001 From: Hlib Suslov Date: Tue, 3 Nov 2026 09:53:11 +0100 Subject: [PATCH] fix: refresh runtime and harden fetch and scan lifecycles --- .env.example | 1 + .github/workflows/ci.yml | 56 ++++++++++++++++++++----- CHANGELOG.md | 26 ++++++++++++ CONTRIBUTING.md | 7 +++- Dockerfile | 7 ++-- MANIFEST.in | 3 ++ README.md | 27 ++++++++++-- docs/API.md | 6 +++ docs/LINK_GRAPH_INTEGRATION.md | 4 +- docs/SECURITY.md | 12 ++++++ pyproject.toml | 33 ++++++++------- seo_analyzer/__init__.py | 2 +- seo_analyzer/analyzer.py | 21 +++++++++- seo_analyzer/fetcher.py | 44 ++++++++++++++----- seo_analyzer/storage.py | 27 +++++++++--- tests/test_analyzer.py | 32 ++++++++++++++ tests/test_api.py | 3 +- tests/test_fetcher.py | 77 ++++++++++++++++++++++++++++++++++ tests/test_scan_jobs.py | 5 ++- tests/test_storage.py | 65 ++++++++++++++++++++++++++++ 20 files changed, 402 insertions(+), 56 deletions(-) create mode 100644 MANIFEST.in create mode 100644 tests/test_storage.py diff --git a/.env.example b/.env.example index 8c9b537..1ab1c9f 100644 --- a/.env.example +++ b/.env.example @@ -2,6 +2,7 @@ SEO_API_KEY= # Fetch safety and resource budgets. +# Total seconds per fetch, including DNS, queueing, redirects and streaming. SEO_FETCH_TIMEOUT_SECONDS=12 SEO_MAX_RESPONSE_BYTES=3000000 SEO_MAX_REDIRECTS=5 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 176dd42..541e5da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,16 +8,21 @@ on: permissions: contents: read +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + jobs: test: runs-on: ubuntu-latest + timeout-minutes: 15 strategy: fail-fast: false matrix: - python-version: ["3.11", "3.13", "3.14"] + python-version: ["3.11", "3.12", "3.13", "3.14", "3.15"] steps: - - uses: actions/checkout@v6 - - uses: actions/setup-python@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: ${{ matrix.python-version }} cache: pip @@ -27,30 +32,59 @@ jobs: - run: ruff format --check . - run: mypy seo_analyzer - run: pytest --cov --cov-report=term-missing --cov-report=xml - - if: matrix.python-version == '3.14' + - if: matrix.python-version == '3.15' run: pip-audit package: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - - uses: actions/checkout@v6 - - uses: actions/setup-python@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: - python-version: "3.13" + python-version: "3.15" - run: python -m pip install --upgrade build pip - run: python -m build - run: python -m pip install dist/*.whl - - run: python -c "from seo_analyzer.api import create_app; assert create_app().title" + - name: Verify installed wheel outside the source tree + working-directory: ${{ runner.temp }} + run: | + python - <<'PY' + from importlib.metadata import distribution + from seo_analyzer.api import create_app + + assert create_app().title + files = {str(path) for path in distribution("saas-seo-analyzer").files} + for license_file in ("LICENSE", "THIRD_PARTY_NOTICES.md", "licenses/link-graph-explorer-oss.LICENSE"): + assert any(path.endswith("/licenses/" + license_file) for path in files), license_file + PY container: runs-on: ubuntu-latest + timeout-minutes: 15 steps: - - uses: actions/checkout@v6 - - uses: docker/setup-buildx-action@v4 - - uses: docker/build-push-action@v7 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + - uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4 + - uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7 with: context: . push: false + load: true tags: saas-seo-analyzer:test cache-from: type=gha cache-to: type=gha,mode=max + - name: Verify container health and bundled licenses + run: | + docker run --detach --name analyzer --publish 127.0.0.1:8000:8000 \ + --read-only --tmpfs /tmp --cap-drop ALL --security-opt no-new-privileges \ + saas-seo-analyzer:test + trap 'docker logs analyzer; docker rm --force --volumes analyzer' EXIT + for attempt in $(seq 1 30); do + if curl --fail --silent http://127.0.0.1:8000/readyz; then + break + fi + sleep 1 + done + curl --fail http://127.0.0.1:8000/healthz + curl --fail http://127.0.0.1:8000/readyz + docker exec analyzer python -c "from importlib.metadata import distribution; files = {str(p) for p in distribution('saas-seo-analyzer').files}; assert any(p.endswith('/licenses/licenses/link-graph-explorer-oss.LICENSE') for p in files)" diff --git a/CHANGELOG.md b/CHANGELOG.md index fd6f456..d2ee818 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ All notable changes to this fork are documented here. +## 2.0.1 — 2026-11-03 + +### Changed + +- Refreshed runtime and development dependency minimums against current stable + PyPI releases; declared the directly imported Pydantic and Starlette dependencies. +- Added Python 3.12 and 3.15 to CI, covering every supported minor from 3.11 to 3.15. +- Updated the digest-pinned container base to Python 3.15.2, pinned CI actions by + commit, and added workflow cancellation and job timeouts. +- CI now checks the installed wheel outside the checkout and starts the hardened + container to verify health, readiness, and third-party attribution files. +- Updated Windows setup, timeout behavior, scan lifecycle, and contributor docs. + +### Fixed + +- Explicitly close SQLite connections after commit or rollback instead of waiting + for garbage collection; fail tests on resource warnings. +- Keep explicit scan cancellations cancelled during service shutdown. +- Validate scan ownership and publish results in one SQLite write transaction. +- Enforce a total fetch deadline across DNS, queueing, redirects, and streaming. +- Return structured `502` errors for interrupted, malformed, and timed-out response + streams while closing the upstream response. +- Cancel and drain shared in-flight analyses before closing HTTP clients. +- Include the project's and link-graph component's license notices when building + the wheel inside Docker. + ## 2.0.0 — 2026-07-22 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1734c4..57581b8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,7 @@ Contributions should keep the API explainable, safe for caller-controlled public ## Development setup -Python 3.11 or newer is required. +Python 3.11–3.15 is supported. Use a current patch release. ```bash python -m venv .venv @@ -23,7 +23,10 @@ pip-audit python -m build ``` -`make check` runs lint, formatting, types and tests. The CI matrix validates Python 3.11 and 3.13, package installation and the container build. +`make check` runs all six checks above. The CI matrix validates every Python minor +version from 3.11 through 3.15. Separate jobs verify wheel installation outside the +source tree, required license files, and health/readiness of the hardened container. +Resource warnings fail tests so unclosed SQLite connections cannot silently regress. ## Change expectations diff --git a/Dockerfile b/Dockerfile index d8b0fcd..60fa2a4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,14 +1,15 @@ -FROM python:3.13-slim@sha256:6771159cd4fa5d9bba1258caf0b82e6b73458c694d178ad97c5e925c2d0e1a91 AS builder +FROM python:3.15.2-slim@sha256:8c13a3f37a6a5b71498228e902c4b9f51c1f7900d49fabf71f9a650c6120e0d0 AS builder ENV PIP_DISABLE_PIP_VERSION_CHECK=1 \ PIP_NO_CACHE_DIR=1 WORKDIR /build -COPY pyproject.toml README.md ./ +COPY pyproject.toml README.md LICENSE THIRD_PARTY_NOTICES.md ./ +COPY licenses ./licenses COPY seo_analyzer ./seo_analyzer RUN python -m pip wheel --wheel-dir /wheels . -FROM python:3.13-slim@sha256:6771159cd4fa5d9bba1258caf0b82e6b73458c694d178ad97c5e925c2d0e1a91 AS runtime +FROM python:3.15.2-slim@sha256:8c13a3f37a6a5b71498228e902c4b9f51c1f7900d49fabf71f9a650c6120e0d0 AS runtime ENV PYTHONDONTWRITEBYTECODE=1 \ PYTHONUNBUFFERED=1 \ diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 0000000..14ed600 --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,3 @@ +include main.py +include tests/conftest.py +recursive-include docs *.md diff --git a/README.md b/README.md index 172d4ce..33dffd0 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ Interactive OpenAPI docs are available at `/docs` and `/redoc`. ## Quick start -Python 3.11–3.14 is supported. +Python 3.11–3.15 is supported. Use the latest patch release for your Python minor version. ```bash python -m venv .venv @@ -95,6 +95,17 @@ python -m pip install -e '.[dev]' uvicorn main:app --reload ``` +On Windows PowerShell: + +```powershell +python -m venv .venv +.\.venv\Scripts\python.exe -m pip install -e '.[dev]' +.\.venv\Scripts\python.exe -m uvicorn seo_analyzer.api:app --reload +``` + +`seo_analyzer.api:app` also works when running an installed wheel outside the +repository. `main:app` remains the repository's compatibility entry point. + Analyze a page: ```bash @@ -138,7 +149,7 @@ If `SEO_API_KEY` is set, add `-H 'X-API-Key: …'` to protected endpoints. ## Docker -The image runs as a non-root user. The Compose example binds only to loopback, +The image uses a digest-pinned Python 3.15.2 base and runs as a non-root user. The Compose example binds only to loopback, drops Linux capabilities, uses a read-only root filesystem, and persists SQLite in the `analyzer-data` volume mounted at `/data`. @@ -154,7 +165,7 @@ All settings use the `SEO_` prefix. See [`.env.example`](.env.example) for the c | Variable | Default | Meaning | |---|---:|---| | `SEO_API_KEY` | empty | Optional `X-API-Key` shared secret | -| `SEO_FETCH_TIMEOUT_SECONDS` | `12` | Upstream request timeout | +| `SEO_FETCH_TIMEOUT_SECONDS` | `12` | Total fetch deadline, including DNS, concurrency queue, redirects, and response streaming | | `SEO_MAX_RESPONSE_BYTES` | `3000000` | Maximum decompressed page/resource body | | `SEO_MAX_REDIRECTS` | `5` | Redirect budget | | `SEO_MAX_CONCURRENT_FETCHES` | `8` | Process-wide fetch concurrency | @@ -182,17 +193,25 @@ ruff format --check . mypy seo_analyzer pytest --cov --cov-report=term-missing pip-audit +python -m build ``` The suite covers URL security, DNS/redirect validation, parsing, page classification, issue scoring, sitemap/robots behavior, crawl aggregation, PageSpeed normalization, API compatibility, auth, and opportunity ranking. +CI tests every Python minor version from 3.11 through 3.15, installs the built +wheel outside the source tree, and starts the hardened container to check health, +readiness, and bundled attribution files. Dependabot checks Python dependencies, +pinned GitHub Actions, and the Docker base weekly. + ## Scan data SQLite stores projects, scans, normalized page URLs, stable graph node IDs, links with page zones, SEO reports, status/depth/timing metadata, and the complete versioned result snapshot. HTML response bodies are not persisted. Running scans write a worker lease and heartbeat; graceful shutdown requeues active work, while -an abandoned lease is recovered automatically. +an abandoned lease is recovered automatically. An explicit cancellation remains +cancelled during shutdown. Result publication checks ownership in the same SQLite +write transaction, and each database operation closes its connection explicitly. ## Known limitations diff --git a/docs/API.md b/docs/API.md index a9b5f97..f73415c 100644 --- a/docs/API.md +++ b/docs/API.md @@ -38,6 +38,12 @@ Common statuses are: FastAPI validation errors use its standard `detail` array. Clients should branch on the HTTP status first and tolerate additional response fields. +`SEO_FETCH_TIMEOUT_SECONDS` bounds the complete fetch, including DNS, waiting for +a fetch slot, redirects, and streaming the body. Each standalone DNS validation is +also bounded. A stalled DNS lookup returns `dns_resolution_failed`; an expired +fetch deadline or an interrupted/malformed response returns `upstream_unreachable` +with HTTP `502`. Client cancellation is propagated to the caller. + ## `GET /v1/analyze` Audits one page. diff --git a/docs/LINK_GRAPH_INTEGRATION.md b/docs/LINK_GRAPH_INTEGRATION.md index 657d7a9..0b536d7 100644 --- a/docs/LINK_GRAPH_INTEGRATION.md +++ b/docs/LINK_GRAPH_INTEGRATION.md @@ -34,7 +34,9 @@ adds persistent link-graph scans on top of the existing safe asynchronous ## Limitations - Active work is executed in-process. Graceful shutdown immediately requeues it; - process crashes are recovered after `SEO_SCAN_JOB_LEASE_SECONDS`. + explicitly cancelled scans remain cancelled. Process crashes are recovered after + `SEO_SCAN_JOB_LEASE_SECONDS`. Result publication and worker ownership checks + share a SQLite write transaction. - External links are collected and visualized, but external targets are not fully crawled. - The dashboard is intentionally dependency-free and compact; deeper graph diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 22ec5d9..74bd443 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -17,6 +17,11 @@ Before every request, including redirects, the fetcher: Checking every DNS answer is intentionally conservative: a hostname with mixed public/private answers is rejected. Pinning the selected answer for the connection reduces time-of-check/time-of-use DNS-rebinding exposure. +`SEO_FETCH_TIMEOUT_SECONDS` is an overall deadline covering DNS, concurrency +queueing, all redirects, and response streaming. HTTPX's per-operation timeouts +remain in place. A server sending occasional small chunks cannot keep a fetch +running indefinitely. See [HTTPX timeout semantics](https://www.python-httpx.org/advanced/timeouts/). + `SEO_ALLOW_PRIVATE_HOSTS=true` disables the central address boundary and should only be used in a trusted, isolated deployment with an explicit egress policy. Never enable it on a public multi-tenant endpoint. ## Crawl controls @@ -51,6 +56,13 @@ bodies are not intentionally persisted. Multi-worker deployments have independen caches and metrics, while scan ownership and cancellation are coordinated through SQLite worker leases. +SQLite transaction scopes also explicitly close their connections; the standard +connection context manager alone only commits or rolls back. Result publication +acquires a write transaction before validating worker ownership, preventing a +cancelled or replaced worker from overwriting the current scan. Explicitly +cancelled scans are not requeued during graceful shutdown. See the +[SQLite connection context manager](https://docs.python.org/3/library/sqlite3.html#how-to-use-the-connection-context-manager). + ## Container posture The supplied image runs as a non-root system user. Compose binds to loopback, diff --git a/pyproject.toml b/pyproject.toml index 929920b..73c01fe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,10 +1,10 @@ [build-system] -requires = ["setuptools>=77"] +requires = ["setuptools>=84.2.1"] build-backend = "setuptools.build_meta" [project] name = "saas-seo-analyzer" -version = "2.0.0" +version = "2.0.1" description = "Security-first SEO and growth-strategy analyzer for SaaS websites" readme = "README.md" requires-python = ">=3.11" @@ -20,28 +20,31 @@ classifiers = [ "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", "Programming Language :: Python :: 3.14", + "Programming Language :: Python :: 3.15", "Topic :: Internet :: WWW/HTTP :: Indexing/Search", ] dependencies = [ - "beautifulsoup4>=4.14.3,<5", + "beautifulsoup4>=4.15,<5", "defusedxml>=0.7.1,<1", - "fastapi>=0.139.2,<1", + "fastapi>=0.162,<1", "httpx>=0.28.1,<1", - "prometheus-client>=0.25,<1", - "pydantic-settings>=2.14.2,<3", - "uvicorn>=0.51,<1", + "prometheus-client>=0.26,<1", + "pydantic>=2.14.2,<3", + "pydantic-settings>=2.16.2,<3", + "starlette>=1.10,<2", + "uvicorn>=0.54,<1", ] [project.optional-dependencies] dev = [ - "build>=1.5,<2", - "httpx2>=2.7,<3", - "mypy>=2.3,<3", + "build>=1.6.1,<2", + "httpx2>=2.16,<3", + "mypy>=2.4.1,<3", "pip-audit>=2.10.1,<3", - "pytest>=9.1.1,<10", - "pytest-asyncio>=1.4,<2", - "pytest-cov>=7,<8", - "ruff>=0.15.22,<1", + "pytest>=9.2,<10", + "pytest-asyncio>=1.4.1.post1,<2", + "pytest-cov>=7.1,<8", + "ruff>=0.17.3,<1", ] [project.urls] @@ -54,6 +57,8 @@ include = ["seo_analyzer*"] [tool.pytest.ini_options] addopts = "-ra --strict-config --strict-markers" asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" +filterwarnings = ["error::ResourceWarning", "error::pytest.PytestUnraisableExceptionWarning"] testpaths = ["tests"] [tool.coverage.run] diff --git a/seo_analyzer/__init__.py b/seo_analyzer/__init__.py index 3ee6cd6..90c7df2 100644 --- a/seo_analyzer/__init__.py +++ b/seo_analyzer/__init__.py @@ -1,3 +1,3 @@ """SaaS-focused SEO analysis engine.""" -__version__ = "2.0.0" +__version__ = "2.0.1" diff --git a/seo_analyzer/analyzer.py b/seo_analyzer/analyzer.py index 8a2a628..b4459b5 100644 --- a/seo_analyzer/analyzer.py +++ b/seo_analyzer/analyzer.py @@ -34,10 +34,23 @@ def __init__( self._inflight_lock = asyncio.Lock() self._inflight: dict[tuple[str, bool, bool], asyncio.Task[AnalysisArtifact]] = {} self._cleanup_tasks: set[asyncio.Task[None]] = set() + self._closed = False async def close(self) -> None: - await self.fetcher.close() - await self.pagespeed.close() + async with self._inflight_lock: + self._closed = True + tasks = list(self._inflight.values()) + for task in tasks: + task.cancel() + if tasks: + await asyncio.gather(*tasks, return_exceptions=True) + # Flush completion callbacks even when every task was already done. + await asyncio.sleep(0) + while self._cleanup_tasks: + cleanup = list(self._cleanup_tasks) + await asyncio.gather(*cleanup, return_exceptions=True) + self._cleanup_tasks.difference_update(cleanup) + await asyncio.gather(self.fetcher.close(), self.pagespeed.close()) async def analyze( self, @@ -60,6 +73,8 @@ async def analyze_artifact( include_pagespeed: bool = False, include_subdomains: bool = False, ) -> AnalysisArtifact: + if self._closed: + raise RuntimeError("The analyzer is closed") parsed_url, _ = await self.fetcher.validate(url) key = ( normalize_url(str(parsed_url), keep_query=True), @@ -71,6 +86,8 @@ async def analyze_artifact( return cached async with self._inflight_lock: + if self._closed: + raise RuntimeError("The analyzer is closed") task = self._inflight.get(key) if task is None: task = asyncio.create_task( diff --git a/seo_analyzer/fetcher.py b/seo_analyzer/fetcher.py index 668931d..43ec53c 100644 --- a/seo_analyzer/fetcher.py +++ b/seo_analyzer/fetcher.py @@ -113,11 +113,17 @@ async def validate(self, url: str) -> tuple[httpx.URL, list[IPAddress]]: literal_address = ipaddress.ip_address(parsed.host) except ValueError: literal_address = None - addresses = ( - [literal_address] - if literal_address is not None - else await self._resolver(parsed.host, port) - ) + try: + async with asyncio.timeout(self.settings.fetch_timeout_seconds): + addresses = ( + [literal_address] + if literal_address is not None + else await self._resolver(parsed.host, port) + ) + except TimeoutError as exc: + raise FetchError( + "dns_resolution_failed", "DNS resolution timed out", status_code=502 + ) from exc if not addresses: raise FetchError( "dns_resolution_failed", "The hostname did not resolve", status_code=502 @@ -136,6 +142,27 @@ async def fetch( *, accepted_content_types: Collection[str] | None = ("text/html", "application/xhtml+xml"), max_bytes: int | None = None, + ) -> FetchResult: + try: + # HTTPX timeouts apply to individual I/O operations. This deadline + # also bounds DNS, queueing, redirects and continuously arriving data. + async with asyncio.timeout(self.settings.fetch_timeout_seconds): + return await self._fetch( + url, accepted_content_types=accepted_content_types, max_bytes=max_bytes + ) + except (TimeoutError, httpx.RequestError) as exc: + raise FetchError( + "upstream_unreachable", + "Upstream request timed out or the response could not be read", + status_code=502, + ) from exc + + async def _fetch( + self, + url: str, + *, + accepted_content_types: Collection[str] | None, + max_bytes: int | None, ) -> FetchResult: limit = min(max_bytes or self.settings.max_response_bytes, self.settings.max_response_bytes) parsed_input, _ = await self.validate(url) @@ -195,12 +222,7 @@ async def _request_once( extensions={"sni_hostname": public_url.host}, ) started_at = time.perf_counter() - try: - response = await self._client.send(request, stream=True) - except (httpx.TimeoutException, httpx.NetworkError, httpx.RemoteProtocolError) as exc: - raise FetchError( - "upstream_unreachable", f"Upstream request failed: {exc}", status_code=502 - ) from exc + response = await self._client.send(request, stream=True) headers_at = time.perf_counter() try: declared_length = response.headers.get("content-length") diff --git a/seo_analyzer/storage.py b/seo_analyzer/storage.py index 6d1c0c2..9bb1e02 100644 --- a/seo_analyzer/storage.py +++ b/seo_analyzer/storage.py @@ -2,6 +2,8 @@ import sqlite3 import threading import uuid +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path from typing import Any @@ -22,10 +24,10 @@ def __init__(self, path: str | Path) -> None: self.path.parent.mkdir(parents=True, exist_ok=True) self._lock = threading.RLock() if self._memory: - self._keeper = self.connect() + self._keeper = self._open_connection() self.init_schema() - def connect(self) -> sqlite3.Connection: + def _open_connection(self) -> sqlite3.Connection: conn = sqlite3.connect( self._dsn, check_same_thread=False, @@ -37,6 +39,16 @@ def connect(self) -> sqlite3.Connection: conn.execute("PRAGMA busy_timeout = 10000") return conn + @contextmanager + def connect(self) -> Iterator[sqlite3.Connection]: + """Commit or roll back the transaction, then always close the connection.""" + conn = self._open_connection() + try: + with conn: + yield conn + finally: + conn.close() + def close(self) -> None: if self._keeper is not None: self._keeper.close() @@ -277,12 +289,14 @@ def release_owned_scan(self, scan_id: str, worker_id: str, *, error: str) -> boo cursor = conn.execute( """ UPDATE scans - SET status = 'pending', worker_id = NULL, heartbeat_at = NULL, + SET status = CASE WHEN status = 'cancelling' THEN 'cancelled' ELSE 'pending' END, + worker_id = NULL, heartbeat_at = NULL, pages_crawled = 0, queued = 0, current_url = '', - error = ?, finished_at = NULL + error = CASE WHEN status = 'cancelling' THEN NULL ELSE ? END, + finished_at = CASE WHEN status = 'cancelling' THEN ? ELSE NULL END WHERE id = ? AND worker_id = ? AND status IN ('running', 'cancelling') """, - (error, scan_id, worker_id), + (error, utc_now_iso(), scan_id, worker_id), ) return cursor.rowcount == 1 @@ -361,6 +375,9 @@ def save_result( ) -> bool: pages = result.get("crawl", {}).get("pages", {}) with self._lock, self.connect() as conn: + # Ownership validation and publication must share a write transaction + # across API processes, including cancellation and lease recovery. + conn.execute("BEGIN IMMEDIATE") if worker_id: owner = conn.execute( "SELECT 1 FROM scans WHERE id = ? AND worker_id = ? AND status = 'running'", diff --git a/tests/test_analyzer.py b/tests/test_analyzer.py index 485b355..05e00ae 100644 --- a/tests/test_analyzer.py +++ b/tests/test_analyzer.py @@ -123,3 +123,35 @@ async def handler(request: httpx.Request) -> httpx.Response: assert analyzer._inflight == {} assert report.score.overall >= 90 assert calls == 1 + + +@pytest.mark.asyncio +async def test_close_cancels_shared_work_before_closing_clients() -> None: + started = asyncio.Event() + stopped = asyncio.Event() + + async def handler(request: httpx.Request) -> httpx.Response: + started.set() + try: + await asyncio.Event().wait() + finally: + stopped.set() + + settings = make_settings() + analyzer = Analyzer(settings, fetcher=make_fetcher(handler, settings=settings)) + waiting = asyncio.create_task(analyzer.analyze("https://saas.test")) + try: + await asyncio.wait_for(started.wait(), timeout=1) + await analyzer.close() + assert stopped.is_set() + with pytest.raises(asyncio.CancelledError): + await waiting + assert analyzer._inflight == {} + assert not analyzer._cleanup_tasks + finally: + waiting.cancel() + await asyncio.gather(waiting, return_exceptions=True) + for task in list(analyzer._inflight.values()): + task.cancel() + await asyncio.gather(*list(analyzer._inflight.values()), return_exceptions=True) + await analyzer.close() diff --git a/tests/test_api.py b/tests/test_api.py index 358539f..9866072 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -5,6 +5,7 @@ from conftest import OPTIMIZED_HTML, make_fetcher, make_settings from fastapi.testclient import TestClient +from seo_analyzer import __version__ from seo_analyzer.analyzer import Analyzer from seo_analyzer.api import create_app @@ -46,7 +47,7 @@ def test_operations_analysis_and_legacy_endpoints() -> None: with client: root = client.get("/") assert root.status_code == 200 - assert root.json()["version"] == "2.0.0" + assert root.json()["version"] == __version__ assert client.get("/healthz").json()["status"] == "ok" assert client.get("/readyz").json()["status"] == "ready" metrics = client.get("/metrics") diff --git a/tests/test_fetcher.py b/tests/test_fetcher.py index 41a65bb..ac7dcae 100644 --- a/tests/test_fetcher.py +++ b/tests/test_fetcher.py @@ -1,3 +1,4 @@ +import asyncio import ipaddress import httpx @@ -194,3 +195,79 @@ async def empty_resolver(_hostname: str, _port: int) -> list: @pytest.mark.asyncio async def test_static_resolver_handles_literal_ip() -> None: assert await SafeFetcher._resolve(PUBLIC_IP, 443) == [ipaddress.ip_address(PUBLIC_IP)] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "error_type", + [httpx.ReadTimeout, httpx.ReadError, httpx.RemoteProtocolError, httpx.DecodingError], +) +async def test_stream_failures_are_normalized_and_closed(error_type) -> None: + class BrokenStream(httpx.AsyncByteStream): + closed = False + + async def __aiter__(self): + yield b"" + raise error_type("broken response") + + async def aclose(self): + self.closed = True + + stream = BrokenStream() + fetcher = make_fetcher( + lambda request: httpx.Response(200, headers={"content-type": "text/html"}, stream=stream) + ) + try: + with pytest.raises(FetchError) as caught: + await fetcher.fetch("https://public.test") + assert caught.value.code == "upstream_unreachable" + assert caught.value.status_code == 502 + assert stream.closed + finally: + await fetcher.close() + + +@pytest.mark.asyncio +async def test_total_deadline_stops_a_continuously_streaming_response() -> None: + class SlowStream(httpx.AsyncByteStream): + closed = False + + async def __aiter__(self): + while True: + await asyncio.sleep(0.005) + yield b"x" + + async def aclose(self): + self.closed = True + + stream = SlowStream() + settings = make_settings().model_copy(update={"fetch_timeout_seconds": 0.05}) + fetcher = make_fetcher( + lambda request: httpx.Response(200, headers={"content-type": "text/html"}, stream=stream), + settings=settings, + ) + try: + with pytest.raises(FetchError) as caught: + await asyncio.wait_for(fetcher.fetch("https://public.test"), timeout=1) + assert caught.value.code == "upstream_unreachable" + assert stream.closed + finally: + await fetcher.close() + + +@pytest.mark.asyncio +async def test_validation_has_a_dns_deadline() -> None: + async def stalled_resolver(_hostname, _port): + await asyncio.Event().wait() + + settings = make_settings().model_copy(update={"fetch_timeout_seconds": 0.05}) + fetcher = make_fetcher( + lambda request: httpx.Response(200), settings=settings, resolver=stalled_resolver + ) + try: + with pytest.raises(FetchError) as caught: + await asyncio.wait_for(fetcher.validate("https://public.test"), timeout=1) + assert caught.value.code == "dns_resolution_failed" + assert caught.value.status_code == 502 + finally: + await fetcher.close() diff --git a/tests/test_scan_jobs.py b/tests/test_scan_jobs.py index ffb68d0..f8d61c2 100644 --- a/tests/test_scan_jobs.py +++ b/tests/test_scan_jobs.py @@ -67,7 +67,8 @@ async def fake_crawl(*_args, should_cancel, **_kwargs): @pytest.mark.asyncio -async def test_cancel_is_observed_across_managers(tmp_path, monkeypatch) -> None: +@pytest.mark.parametrize("shutdown_owner", [False, True]) +async def test_cancel_is_observed_across_managers(tmp_path, monkeypatch, shutdown_owner) -> None: started = asyncio.Event() async def fake_crawl(*_args, should_cancel, **_kwargs): @@ -88,6 +89,8 @@ async def fake_crawl(*_args, should_cancel, **_kwargs): scan = owner.create_and_start(project["id"], project["root_url"], UnifiedCrawlOptions()) await asyncio.wait_for(started.wait(), timeout=1) assert peer.cancel(scan["id"]) is True + if shutdown_owner: + await owner.shutdown() for _ in range(100): if storage.get_scan(scan["id"])["status"] == "cancelled": break diff --git a/tests/test_storage.py b/tests/test_storage.py new file mode 100644 index 0000000..7a09135 --- /dev/null +++ b/tests/test_storage.py @@ -0,0 +1,65 @@ +import sqlite3 + +import pytest + +from seo_analyzer.storage import ScanStorage + + +def test_connections_commit_rollback_and_close(tmp_path) -> None: + storage = ScanStorage(tmp_path / "storage.db") + try: + with storage.connect() as committed: + committed.execute( + "INSERT INTO projects VALUES (?, ?, ?, ?)", + ("saved", "Saved", "https://site.test/", "2026-11-03T00:00:00Z"), + ) + assert storage.get_project("saved") is not None + with pytest.raises(sqlite3.ProgrammingError, match="closed"): + committed.execute("SELECT 1") + + with pytest.raises(ValueError, match="abort"), storage.connect() as rolled_back: + rolled_back.execute( + "INSERT INTO projects VALUES (?, ?, ?, ?)", + ("aborted", "Aborted", "https://site.test/", "2026-11-03T00:00:00Z"), + ) + raise ValueError("abort") + assert storage.get_project("aborted") is None + with pytest.raises(sqlite3.ProgrammingError, match="closed"): + rolled_back.execute("SELECT 1") + finally: + storage.close() + + +def test_shutdown_preserves_persisted_cancellation() -> None: + storage = ScanStorage(":memory:") + try: + project = storage.create_project("https://site.test/") + scan = storage.create_scan(project["id"], project["root_url"], {}) + assert storage.claim_scan(scan["id"], "owner") + assert storage.request_cancel(scan["id"]) + assert storage.release_owned_scan(scan["id"], "owner", error="shutdown") + saved = storage.get_scan(scan["id"]) + assert saved["status"] == "cancelled" + assert saved["finished_at"] is not None + assert saved["error"] is None + assert storage.recover_stale_scans("2099-01-01T00:00:00Z") == [] + finally: + storage.close() + + +def test_stale_worker_cannot_publish_a_result() -> None: + storage = ScanStorage(":memory:") + try: + project = storage.create_project("https://site.test/") + scan = storage.create_scan(project["id"], project["root_url"], {}) + assert storage.claim_scan(scan["id"], "old-worker") + storage.recover_stale_scans("2099-01-01T00:00:00Z") + assert storage.claim_scan(scan["id"], "new-worker") + result = {"crawl": {"pages": {}}, "stats": {"total_pages": 0}} + assert storage.save_result(scan["id"], result, worker_id="old-worker") is False + assert storage.get_scan(scan["id"])["status"] == "running" + assert storage.get_result(scan["id"]) is None + assert storage.save_result(scan["id"], result, worker_id="new-worker") is True + assert storage.get_scan(scan["id"])["status"] == "completed" + finally: + storage.close()