Skip to content

Empty searches exit 4, partial concurrent runs say so, one paid solve per page - #7

Merged
jehrr merged 1 commit into
mainfrom
fix/audit-2026-10-14
Oct 15, 2026
Merged

jehrr merged 1 commit into
mainfrom
fix/audit-2026-10-14

Conversation

@jehrr

@jehrr jehrr commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Fixes from the 2026-10-14 audit of cb7123b. main has not moved since that audit.

Problems and resulting behaviour

Finding Before After
H1: concurrency --pages 4 --concurrency 2 with every worker failing to open a session: exit 0, status complete, pages_failed [], 24 rows Every planned page is walked in order. Pages that no worker fetched go into pages_failed and the new sidecar key pages_unattempted, and the run is partial (exit 6).
H2: empty search search-count:0 → finish_run(..., pages_failed=[1]) → exit 5 in all three browser engines A served empty page is passed with pages_failed=[] and pages_completed=1, and no_results is a complete stop reason → exit 4. load_failed and parser_found_nothing are still exit 5.
H3: captcha budget No budget. --retries 2 on a gated page made 3 solver calls (Playwright, Selenium) page_flow.SolveBudget with SOLVES_PER_PAGE = 1. It is checked immediately before solve_recaptcha and is a required keyword-only argument of handle_captcha_if_present in every engine. It is not reset by retry or rotation, and each concurrency worker's session open gets its own allowance. Result: 1 solve.
M1: atomic writes An interrupted json.dump left out.json as 12 bytes of invalid JSON _atomic writes a temp file in the target's directory, then flush, fsync, os.replace, with unlink on error and newline="" kept for CSV. Applies to JSON, CSV and the sidecar. A new file gets 0o666 & ~umask; an existing file keeps its mode.
M2: CSV formulas Cells written unchanged String cells starting with = + - @ TAB CR LF (checked after list joining) get a leading ' in CSV only. Numbers are untouched. csv_cells_escaped goes into the sidecar, merged under the caller's extra.
M3: retries -1 Passed argparse, made 0 navigations, exit 3 --retries, --delay and --retry-delay must be ≥ 0, and --pages and --concurrency must be ≥ 1. Anything else is an argparse error naming the flag (exit 2). This applies in all engines and in scraper_api_client.py. --retries 0 still navigates once.
M4: nav failure Selenium and pyppeteer initialised state="blocked", so a dead network exited 3 Both now start at load_failed and set it on every navigation exception, so this case exits 5, as Playwright already did.
M5: rotation (verified by reading, then by test) pool.advance() without a relaunch, so later attempts stayed on the old exit The browser is quit and relaunched on the new exit, on proxy failure and on a retryable page-1 state (as Playwright does)
M6: source_changed Documented but missing: an API price against a DOM price showed up as changed A price difference that comes with a price_source difference is reported as source_changed and ignored by --fail-on-change
M7: donor residue CONTRIBUTING, bug_report.yml, site_changed.yml and the claude-code-review.yml prompt described another site All four rewritten for Indiegogo
M8: credential scan A clone under venv/ scanned 0 files and reported "ok" Paths are checked as path.relative_to(REPO).parts, and an empty scan fails. The scan now also reads .json/.csv/.toml/.cfg/.sh. The pyvenv.cfg skip still works.
M9: private citations 16 references in shipped files All removed and restated in this repo's own words. A suite guard was added; its tokens are assembled from pieces so the guard does not match itself. The v0.1.0 release notes had none, so they were not edited.
L2 --proxy-block-retries had no consumer Help text now says the flag is accepted for compatibility and has no effect here
L4 "credentials expire after about a day" Now: credentials expire, and a 401 means fetch a fresh endpoint
L5 README said all four entry points share flags; .gitignore named files that do not exist Both statements corrected
L6 / canary Exit 3 from a bare runner was a green ::notice:: skip, and the exit 4/5 messages were stale See the Canary section below

Evidence

  • Offline suite: 113 passed, 0 failed, run in four venvs:

    • core only: 3 skips, as expected;
    • requirements-playwright.txt: 2 skips;
    • requirements-selenium.txt: 2 skips;
    • requirements-puppeteer.txt: 2 skips.

    In each engine venv, that engine's behavioural checks ran (no browser was downloaded).

  • New checks:

    • atomic writes, with a planted failure in json.dump, writerow and the sidecar;
    • modes under umask 022, 002 and 077, plus a kept 0640;
    • CSV escaping and the csv_cells_escaped merge order;
    • exit 4 for empty, exit 5 for load failure and for parser failure;
    • SolveBudget unit test, and its wiring (read from disk);
    • numeric validators, read from disk and through each importable engine's real parser;
    • fake-driver behaviour for Playwright: one solve, and missing concurrent pages → exit 6;
    • fake-driver behaviour for Selenium and pyppeteer: one solve, nav failure → 5, rotation relaunches on a new exit;
    • diff_runs source_changed;
    • repo-relative scan;
    • no private citations.
  • Planted-fault controls: 19, all red on the named check. Each was run with PYTHONDONTWRITEBYTECODE=1 and the cache removed between runs.

    • _atomic → open()
    • mode → 0o644
    • escape disabled
    • extra merge reversed
    • no_results removed from the complete list
    • pages_failed=[1] restored
    • budget check removed (core and selenium venv)
    • budget bypassed in Playwright and pyppeteer
    • missing-page accounting removed
    • --retries type=int
    • Selenium and pyppeteer state="blocked" on nav failure
    • Selenium and pyppeteer rotation without relaunch
    • source_changed disabled
    • absolute path.parts
    • citation planted in the README
  • Real control for the scan: a copy of the repo under …/venv/clone with a planted 32-hex string → FAILED: [secret] planted.py:1. A pyvenv.cfg directory inside the clone is still skipped (39 files scanned, clean).

  • ci_checks.py --all: help, sample and secret checks all OK.

  • No paid calls, proxies, credentials or live requests. Every solver call in the tests is a stub.

Canary

  • Before: daily, no secrets configured (gh secret list is empty). The scheduled runs on 2026-10-06 and from 2026-10-13 to 2026-10-14 were real bare-runner runs with 72 rows on 3/3 pages (artifacts checked). The runs from 2026-10-07 to 2026-10-12 were red because of the old pct_funded check, which is fixed on main. Exit 3 from a bare runner was skipped green.
  • After: no secret is required, because the README's claim is exactly that this path needs none. Any of the following is red:
    • exit 3, 4, 5 or 6;
    • fewer than 48 rows;
    • a core field below its floor.
  • Dispatch result: see the comment below.

Not changed

  • Version bump, tag and release. The behaviour changes are under [Unreleased].
  • T10: shared fetch loop refactor (owner decision).
  • L3: pyppeteer page.authenticate, its hardcoded UA, and Selenium's hint-less UA override. These need a live authenticated-proxy check.
  • L7: scanning gitignored untracked files. The report marks this an owner decision; .env* is already gitignored.
  • L8: refusing --fingerprint together with --cdp-endpoint. Low severity, and it would be another behaviour change.
  • L1: an empty page inside the site's stated total. It is unknown whether the site ever returns one (open question 1).
  • .env.example and builder flags: already compliant (INDIEGOGO_URL= is empty, and there are no URL-building flags).
  • Repo metadata: the description already names "2Captcha Scraping Browser API", and the .github audit lists nothing for this repo.

🤖 Generated with Claude Code

… per page

- A search the site answers with search-count:0 exits 4 in every browser
  engine (it exited 5); a --concurrency run whose workers never fetched some
  planned pages is partial (exit 6) with pages_unattempted, not complete.
- Paid captcha solves go through one per-page budget (page_flow.SolveBudget),
  checked before the purchase; handle_captcha_if_present requires it.
- Selenium/pyppeteer: a dead network exits 5 instead of 3, and proxy
  rotation relaunches the browser on the new exit.
- --retries/--delay/--retry-delay < 0 and --pages/--concurrency < 1 are
  usage errors naming the flag, in all engines and scraper_api_client.
- Output, CSV and sidecar are written atomically with umask-honouring modes;
  CSV neutralises formula-shaped text cells and records csv_cells_escaped.
- diff_runs implements the documented source_changed bucket.
- The credential scan tests repo-relative paths, fails on an empty scan, and
  reads .json/.csv/.toml/.cfg/.sh.
- CONTRIBUTING, issue templates and the review prompt describe Indiegogo;
  exit-code docs match the code; private-notes citations removed, with a
  suite guard; Scraping Browser lifetime claim corrected.
- Canary: a block from a bare runner is now a failure, not a green skip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jehrr

jehrr commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

CI on head 626773d: tests ✅, Claude Code Review ✅. Canary dispatched on this branch: https://github.com/2scraper/indiegogo-scraper/actions/runs/38695975242 — success, bare runner with no secrets, 72 rows, 3/3 pages, status: complete, csv_cells_escaped: 0.

@jehrr
jehrr marked this pull request as ready for review October 15, 2026 13:02
@jehrr
jehrr merged commit 06a1bb6 into main Oct 15, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant