refactor: layered scrape API, timeout_phase telemetry, and isolation hardening

[gildesmarais]

What changed

• Split monolithic app/main.py, app/engine.py, and app/schemas.py into layered packages: api/ (routes, deps, errors, OpenAPI), domain/scrape_service.py, engine/ (orchestrator, tiers, strategies, envelope, budget.py), schemas/, infra/, and security/url_guard.py.
• create_app() lifespan owns one shared ScraperEngine and ThreadPoolExecutor on app.state; per-request isolation stays on runtime dirs, browser profiles, and drivers.
• Route modules expose create_router() factories included after configure_openapi(settings), so timeout-dependent response metadata is not frozen on first import; wait_timeout_seconds uses a settings-aware default_factory with a stable OpenAPI default of 15.
• ScrapeService.process() owns request-id resolution and SSRF guardrails; /scrape is a thin call plus json_response().
• Added diagnostics.timeout_phase (queue | boot | work) on timeout outcomes via app/infra/scrape_progress.py; engine marks phases at queue/boot/work boundaries; 504 handler and Sentry tags read the last snapshot.
• Wall-clock budget math lives in app/engine/budget.py; orchestrator derives start from the API submission deadline so queue wait cannot grant a second full work budget.
• Session/boot hardening: unregister active request id on prepare failure; prune orphan runtime dirs under the active-id lock; map Driver() boot failures to the 502 envelope; ENOSPC on profile creation removes the partial tree before prune-and-retry.
• Optional Botasaurus methods go through driver_capabilities.call_if_available / resolve_callable; lazy Botasaurus/CDP imports in tier entrypoints.
• Typed Sentry vendor seam (SentryScope protocol); single readiness fact; single logger owner via app.logging_config.get_logger().
• OpenAPI examples from Pydantic instances in app/api/openapi_examples.py; Settings threaded through deps (no import-time freezes).
• Deleted dead surfaces (ScrapeService.serialize(), scripts/xhr_spike.py, unused params, duplicate Sentry readiness checks).
• Reorganized tests by layer (tests/api/, tests/domain/, tests/engine/, tests/infra/, tests/security/) with shared fakes in tests/support/; HTTP contract pins SSRF 403 at the in-process seam.
• Added make typecheck (pyright strict on app + tests) to make check and CI (requirements-dev.txt + .github/workflows/ci.yml); updated AGENTS.md and docs/typing-residuals.md.

Why

The codebase had grown into large monolith modules that mixed HTTP wiring, scrape orchestration, and engine internals. Shallow pass-through surfaces (route-level validation, duplicated budget math, scattered logger names) made isolation guarantees hard to audit and blocked strict typing on driver seams. Ops telemetry could not distinguish queue waits, browser boot, and navigation/work timeouts; queue time previously could still leave a full work budget; stale or half-created runtime dirs on tmpfs could exhaust disk or leave ENOSPC retries non-recreatable.

Risk

• Medium — large internal refactor; public HTTP contract preserved (GET /health, POST /scrape wire shapes, error envelope, status codes). openapi-verify is part of make check; OpenAPI snapshot only has small cosmetic deltas (examples / default docs).
• Singleton engine/executor is process-scoped; multi-worker uvicorn weakens in-process request-id collision detection (documented in AGENTS.md).
• Runtime prune skips dirs tied to active request ids; ENOSPC still surfaces as navigation_error with timeout_phase=boot when retry fails.
• Timeout classification still uses substring matching on exception messages ("timeout" in str(exc)); documented wire behavior, not changed in this branch.

Review map

1. tests/api/test_http_contract.py — SSRF guard 403 pins at the HTTP seam, request-id contract, error envelopes
2. tests/api/test_timeout_http.py + tests/engine/test_timeout_progress.py — timeout_phase at 504 handler and engine tier marks
3. tests/engine/test_isolation.py + tests/engine/test_scraper_engine.py — shared engine/executor vs per-request isolation, collision guard, submission-deadline budget, Driver boot → 502
4. tests/engine/test_budget.py — wall-clock budget clamps (floor at 1s, tighter-constraint min) and timeout-substring classification
5. app/domain/scrape_service.py — process() guardrails, threadpool orchestration, timeout outcome assembly
6. app/engine/budget.py + app/engine/orchestrator.py + app/engine/browser_tier.py + app/engine/request_tier.py — deadline-aware budget and tier execution
7. app/engine/session.py — prepare failure unregister, lock-scoped prune, ENOSPC partial-dir removal before retry
8. app/main.py + app/api/ — create_app() lifespan, create_router() after configure_openapi, thin routes, OpenAPI metadata

Validation

• make check — ruff, hadolint, spectral, unit tests, pyright strict, openapi-verify (reported green on this branch)
• CI now installs pyright from requirements-dev.txt and runs typecheck in .github/workflows/ci.yml

[Copilot]

Pull request overview

This PR refactors the scrape service into layered packages while preserving the HTTP contract and adding timeout telemetry, runtime isolation, and stricter typing.

Changes:

• Separates API, domain, engine, schema, infrastructure, and security responsibilities.
• Adds centralized timeout budgets, progress telemetry, cleanup, SSRF guardrails, and typed driver seams.
• Reorganizes tests and updates OpenAPI, documentation, dependencies, and validation tooling.

Reviewed changes

Copilot reviewed 84 out of 90 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Review result
typings/starlette/testclient.pyi	Reviewed; no final comment.
typings/httpx/__init__.pyi	Reviewed; no final comment.
typings/botasaurus/request.pyi	Reviewed; no final comment.
typings/botasaurus/browser.pyi	Reviewed; no final comment.
typings/botasaurus_driver/core/custom_storage_cdp.pyi	Reviewed; no final comment.
typings/botasaurus_driver/cdp/network.pyi	Reviewed; no final comment.
typings/botasaurus_driver/cdp/__init__.pyi	Reviewed; no final comment.
typings/botasaurus_driver/__init__.pyi	Reviewed; no final comment.
tests/test_bench_regression.py	Reviewed; no final comment.
tests/support/http.py	Reviewed; no final comment.
tests/support/fakes.py	Reviewed; no final comment.
tests/support/factories.py	Reviewed; no final comment.
tests/support/__init__.py	Reviewed; no final comment.
tests/security/test_url_guard.py	Reviewed; no final comment.
tests/security/__init__.py	Reviewed; no final comment.
tests/infra/test_xhr_collector.py	Reviewed; no final comment.
tests/infra/test_sentry.py	Reviewed; no final comment.
tests/infra/test_scrape_progress.py	Reviewed; no final comment.
tests/infra/test_runtime_cleanup.py	Reviewed; no final comment.
tests/infra/test_request_id.py	Reviewed; no final comment.
tests/infra/test_ops_telemetry.py	Reviewed; no final comment.
tests/infra/test_metadata_extractor.py	Reviewed; no final comment.
tests/infra/test_challenge_detector.py	Reviewed; no final comment.
tests/infra/__init__.py	Reviewed; no final comment.
tests/engine/test_timeout_progress.py	Reviewed; no final comment.
tests/engine/test_scraper_engine.py	Reviewed; no final comment.
tests/engine/test_isolation.py	Reviewed; no final comment.
tests/engine/test_budget.py	Reviewed; no final comment.
tests/engine/__init__.py	Reviewed; no final comment.
tests/domain/test_timeout_error.py	Reviewed; no final comment.
tests/domain/__init__.py	Reviewed; no final comment.
tests/api/test_timeout_http.py	Reviewed; no final comment.
tests/api/test_request_schema.py	Reviewed; no final comment.
tests/api/test_http_contract.py	nit (2 votes): Use tests/support/http.test_client() at lines 53, 70, and 151 instead of the module-global app.
tests/api/__init__.py	Reviewed; no final comment.
tests/__init__.py	Reviewed; no final comment.
scripts/xhr_spike.py	Reviewed; no final comment.
scripts/bench_scrape.py	Reviewed; no final comment.
requirements.txt	Reviewed; no final comment.
README.md	Reviewed; no final comment.
pyproject.toml	Reviewed; no final comment.
openapi.yaml	Reviewed; no final comment.
Makefile	critical (1 vote): Ensure clean CI/development installs include Pyright, or make check fails with No module named pyright.
docs/typing-residuals.md	Reviewed; no final comment.
app/sentry.py	Reviewed; no final comment.
app/security/url_guard.py	Reviewed; no final comment.
app/security/__init__.py	Reviewed; no final comment.
app/schemas/response.py	Reviewed; no final comment.
app/schemas/request.py	nit (3 votes): Avoid import-time freezing of the hard-coded 15-second default and timeout description; derive them from live per-app settings.
app/schemas/enums.py	Reviewed; no final comment.
app/schemas/__init__.py	Reviewed; no final comment.
app/schemas.py	Reviewed; no final comment.
app/main.py	Reviewed; no final comment.
app/logging_config.py	Reviewed; no final comment.
app/infra/xhr_collector.py	Reviewed; no final comment.
app/infra/sentry.py	Reviewed; no final comment.
app/infra/scrape_progress.py	Reviewed; no final comment.
app/infra/runtime_cleanup.py	Reviewed; no final comment.
app/infra/request_id.py	Reviewed; no final comment.
app/infra/ops_telemetry.py	Reviewed; no final comment.
app/infra/metadata.py	Reviewed; no final comment.
app/infra/detector.py	Reviewed; no final comment.
app/infra/cdp_types.py	Reviewed; no final comment.
app/infra/__init__.py	Reviewed; no final comment.
app/exceptions.py	Reviewed; no final comment.
app/engine/strategies.py	moderate (3 votes): Route optional-method resolution and invocation through driver_capabilities.call_if_available.
app/engine/session.py	critical (3 votes): Unregister the request ID when runtime preparation fails before __exit__ runs.
app/engine/request_tier.py	Reviewed; no final comment.
app/engine/orchestrator.py	critical (3 votes): Preserve the submission start time for tier budgets after queue waits. critical (1 vote): Synchronize active-request registration with runtime-directory pruning or re-check activity during deletion.
app/engine/envelope.py	Reviewed; no final comment.
app/engine/driver_capabilities.py	Reviewed; no final comment.
app/engine/budget.py	Reviewed; no final comment.
app/engine/browser_tier.py	moderate (3 votes): Catch and map non-timeout driver-construction failures at the browser boot boundary instead of allowing generic 500 responses.
app/engine/__init__.py	Reviewed; no final comment.
app/domain/scrape_service.py	Reviewed; no final comment.
app/domain/__init__.py	Reviewed; no final comment.
app/constants.py	Reviewed; no final comment.
app/config.py	Reviewed; no final comment.
app/api/routes/scrape.py	moderate (2 votes): Defer or factory-scope timeout-dependent response metadata so later configured app instances do not reuse the first import's values.
app/api/routes/health.py	Reviewed; no final comment.
app/api/routes/__init__.py	Reviewed; no final comment.
app/api/openapi.py	Reviewed; no final comment.
app/api/openapi_examples.py	Reviewed; no final comment.
app/api/errors.py	Reviewed; no final comment.
app/api/deps.py	Reviewed; no final comment.
app/api/__init__.py	Reviewed; no final comment.
app/__init__.py	Reviewed; no final comment.
AGENTS.md	Reviewed; no final comment.

Suppressed comments (12)

app/api/errors.py:100

• Unexpected exceptions are encoded with validation_error, so this handler returns HTTP 500 with error_category=validation. That category is documented for 400/422 request validation, while execution failures map to 502; the resulting envelope is misleading and the 500 response is absent from the documented route contract. Use a distinct internal-error response or map scrape execution exceptions to the appropriate execution category/status.

    return json_response(
        validation_error(
            "",
            "Internal server error",
            request_id=request_id,

app/config.py:13

• The outer Settings loads .env, but this nested BaseSettings has its own source configuration and no env_file. Consequently, Sentry values present only in .env are ignored while the other settings load them, despite the documented single environment source. Give SentrySettings the same dotenv source or populate it through the outer settings model.

    model_config = SettingsConfigDict(extra="ignore")

app/domain/scrape_service.py:199

• The engine tiers return ScrapeError(error_category=TIMEOUT) for in-tier and queue timeouts instead of raising, but this branch maps every returned error to HTTP 502. As a result, request-mode/browser tier timeouts are exposed as 502 rather than the documented 504, and terminal telemetry receives the wrong status. Map returned timeout errors to 504 here.

        status_code = 200 if isinstance(result, ScrapeSuccess) else 502

app/engine/browser_tier.py:99

• timeout_phase is documented as present only when error_category is timeout, but this ENOSPC path returns navigation_error with timeout_phase=boot. Clients will misclassify a storage failure as a timeout and the model/README contract is violated. Leave this field null for non-timeout outcomes and update the regression assertion.

            timeout_phase=TimeoutPhase.BOOT,

app/infra/metadata.py:113

• The refactored metadata loop still falls back with metadata_error=None even when extraction fails; the individual extractors also swallow their exceptions. This makes a metadata failure indistinguishable from a successful absence of metadata, contrary to the documented success contract that populates metadata_error while preserving HTML. Preserve the failure detail when returning the fallback result.
  app/main.py:36
• Because app.api.routes is imported only once, its decorators evaluate get_scrape_*_responses() against the first global registry. A later create_app(Settings(...)) updates the top-level metadata but reuses those already-built route objects, so route examples such as the 504 timeout response still contain the first app's settings. Build route metadata per app instead of relying on the module-global registry.
  app/schemas/request.py:184
• This validator reads the cached process-global settings instead of the Settings object threaded into create_app() and ScrapeService. An app created with scrape_work_timeout_seconds=5 can therefore accept wait_timeout_seconds=30 unchanged, leaving request validation/schema behavior inconsistent with that app's configured budget. Make request validation settings-aware or apply the same clamp at the service boundary.
  app/schemas/request.py:27
• The field description calls get_settings() while the model class is imported, freezing the advertised clamp range before create_app(settings=...) can supply its settings. A factory-created app with a different work timeout will expose stale OpenAPI text. Keep the schema description settings-independent or generate it as part of per-app schema configuration.
  scripts/bench_scrape.py:50
• The benchmark records render timings only for 200 responses but still exits successfully and prints wall_ms_p50 when every scrape returns 502/504. The new regression test consequently passes while the endpoint is failing. Treat a non-200 response as a benchmark failure (or make the script explicitly report a failed run).
  tests/api/test_http_contract.py:70
• Use tests/support/http.test_client() here instead of constructing the module-global app directly. The new test convention in AGENTS.md:66 centralizes lifespan and dependency-override cleanup; this second direct TestClient seam should not remain in the reorganized HTTP suite.
  tests/api/test_http_contract.py:155
• This test also bypasses tests/support/http.test_client() and imports the module-global app directly, contrary to AGENTS.md:66. Use the shared helper inside the existing assertLogs context so lifespan and dependency cleanup follow the same path as the other HTTP tests.
  tests/test_bench_regression.py:18
• This regression test runs the benchmark against the default https://example.com URL with no mock or local server. The unit-test suite therefore depends on external DNS/network and can spend roughly two 45-second scrape timeouts when CI is offline; the benchmark script also returns success without asserting a 200 response. Use a deterministic fake/local transport for the test and leave real network benchmarking to smoke/bench jobs.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.