Skip to content

Add robustness and engineering polish#3

Merged
noderaven merged 7 commits into
mainfrom
feature/robustness-and-polish
Jun 25, 2026
Merged

Add robustness and engineering polish#3
noderaven merged 7 commits into
mainfrom
feature/robustness-and-polish

Conversation

@noderaven

Copy link
Copy Markdown
Owner

Summary

  • Reliability: retry with exponential backoff on transient fetch errors, a clear actionable message on HTTP 403, an offline --input-html mode, plus --user-agent and --retries flags and a CLI error guard (exit code 1 instead of a traceback).
  • Packaging: add pyproject.toml (hatchling) exposing a breach-scraper console command; rename package scraper to breach_scraper. Runtime dependencies: none (standard library only).
  • Quality: configure ruff (lint + format), mypy (strict), and bandit; add GitHub Actions CI running all of them plus tests on Python 3.10, 3.11, 3.12, and 3.13.
  • Tests: expand from 2 to 10 cases covering retries, the 403 path, offline parsing, all output formats, --limit, and the error exit code.
  • Docs: expand the README with badges, install/usage (including the new flags), and a development section.

Test Plan

  • CI passes: ruff check, ruff format --check, mypy, bandit, unittest on 3.10-3.13
  • pip install . then breach-scraper --input-html tests/fixtures/wa_atg_sample.html --output json prints the 2 sample rows
  • python -m unittest discover -s tests -> 10 passing

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces offline parsing capabilities, configurable user-agents, and robust retry logic with exponential backoff to the Washington State Attorney General data breach scraper. It also adds a pyproject.toml configuration, updates the documentation, and expands test coverage. The review feedback suggests enhancing error handling by catching HTTPException and ValueError to prevent unhandled tracebacks, importing HTTPException from http.client, handling potential LookupError exceptions during decoding of unrecognized charsets, and capping the exponential backoff sleep duration.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread breach_scraper/wa_atg_scraper.py Outdated
Comment on lines +175 to +176
except (URLError, TimeoutError) as exc:
last_error = exc

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Catch HTTPException in the retry loop. Protocol-level errors like http.client.IncompleteRead or BadStatusLine do not inherit from OSError or URLError but are transient issues that should be retried.

Suggested change
except (URLError, TimeoutError) as exc:
last_error = exc
except (URLError, TimeoutError, HTTPException) as exc:
last_error = exc

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 10ed280. Added HTTPException to the retry loop so transient protocol errors (IncompleteRead, BadStatusLine) are retried. Verified by test_retries_on_http_protocol_error.

Comment thread breach_scraper/wa_atg_scraper.py Outdated
Comment on lines +288 to +290
except (RuntimeError, OSError) as exc:
print(f"error: {exc}", file=sys.stderr)
return 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Catch ValueError (raised by urlopen for malformed/invalid URL schemes) and HTTPException (raised for protocol-level errors) in the main CLI entry point to ensure a clean exit code of 1 instead of a traceback.

Suggested change
except (RuntimeError, OSError) as exc:
print(f"error: {exc}", file=sys.stderr)
return 1
except (RuntimeError, OSError, ValueError, HTTPException) as exc:
print(f"error: {exc}", file=sys.stderr)
return 1

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied ValueError in 10ed280 (a malformed --url such as 'not a url' raises it, and it also covers UnicodeDecodeError from a bad --input-html file). Left HTTPException out here: with the retry-loop change above, HTTPException is converted to RuntimeError before it reaches main, so catching it here would be dead code. Verified by test_malformed_url_returns_error_code.

from html.parser import HTMLParser
from pathlib import Path
from typing import Iterable
from urllib.error import HTTPError, URLError

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Import HTTPException from http.client to allow catching protocol-level exceptions (such as IncompleteRead or BadStatusLine) that can occur during HTTP requests.

Suggested change
from urllib.error import HTTPError, URLError
from http.client import HTTPException
from urllib.error import HTTPError, URLError

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 10ed280. Imported HTTPException from http.client.

Comment thread breach_scraper/wa_atg_scraper.py Outdated
Comment on lines +160 to +162
charset = response.headers.get_content_charset() or "utf-8"
body: bytes = response.read()
return body.decode(charset, errors="replace")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the server returns an invalid or unrecognized charset in the headers, body.decode() will raise a LookupError. Wrapping this in a try-except block and falling back to utf-8 prevents the scraper from crashing on malformed headers.

                charset = response.headers.get_content_charset() or "utf-8"
                body: bytes = response.read()
                try:
                    return body.decode(charset, errors="replace")
                except LookupError:
                    return body.decode("utf-8", errors="replace")

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 10ed280. Wrapped decode in try/except LookupError with a UTF-8 fallback for unknown charsets. Verified by test_unknown_charset_falls_back_to_utf8.

Comment thread breach_scraper/wa_atg_scraper.py Outdated
Comment on lines +177 to +178
if attempt < attempts - 1:
time.sleep(backoff * (2**attempt))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Cap the exponential backoff sleep time to a reasonable maximum (e.g., 30 seconds) to prevent extremely long delays if retries is configured to a high value.

        if attempt < attempts - 1:
            sleep_time = min(backoff * (2**attempt), 30.0)
            time.sleep(sleep_time)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 10ed280. Capped the backoff at 30s with min(backoff * (2**attempt), 30.0). Verified by test_backoff_is_capped.

@noderaven
noderaven merged commit db78b80 into main Jun 25, 2026
4 checks passed
@noderaven
noderaven deleted the feature/robustness-and-polish branch June 25, 2026 00:09
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