alirezamika / alirezamika/autoscraper

Missing default timeout and HTTP status check in _fetch_html can hang builds and mask fetch errors

Abierto
#120 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Python
Estrellas
8k
Forks
826
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

## Summary

`AutoScraper._fetch_html` performs `requests.get(url, ...)` with no default timeout and never checks the HTTP status code. A slow or unresponsive server can therefore hang `build()` / `get_result_*()` indefinitely, and HTTP error pages (404/500/etc.) are silently parsed as if they were valid content.

## Location

- File: `autoscraper/auto_scraper.py`
- Class: `AutoScraper`
- Method: `_fetch_html` (called by `_get_soup`, which is called by `build`, `get_result`, `get_result_similar`, `get_result_exact`)

Relevant code path (static analysis of current `master`):

```python
res = requests.get(url, headers=headers, **request_args)
...
html = res.text
return html
```

No `timeout=` is supplied unless the caller happens to pass one inside `request_args`, and there is no `res.raise_for_status()` / status-code check.

## Problem

1. **No default timeout:** `requests.get` without `timeout` blocks indefinitely by default. Since `_fetch_html` only forwards `timeout` if the user explicitly puts it in `request_args`, the common call `AutoScraper().build(url=...)` has no upper bound on connection/read wait.
2. **HTTP errors silently accepted:** `res.text` is returned for any status code. A 404/403/500 page (often a short error template) is then fed to BeautifulSoup and rule matching, producing confusing empty results instead of a clear error.

## Trigger / Reproduction

- Call `AutoScraper().build(url="https://example.com/slow-endpoint", wanted_list=[...])` where the server accepts the connection but never sends a response body, without passing `request_args={"timeout": ...}`. Based on static analysis, the call will block on `requests.get` with no timeout.
- Call `build()` / `get_result_similar()` against a URL that returns HTTP 404/500. Based on static analysis, the error-page HTML is parsed normally and typically yields `[]`, with no exception or warning indicating the fetch failed.

Note: this is a static-analysis finding; I did not execute the code against a live hanging server.

## Expected Behavior

- Network fetch should have a sane default timeout (e.g. 10-30s) that the caller can override via `request_args`.
- Non-2xx responses should either raise a clear exception or be surfaced to the caller, rather than being parsed as valid pages.

## Actual Behavior

- No timeout is enforced unless the caller knows to inject `request_args={"timeout": ...}`.
- Any HTTP response body, regardless of status, is treated as page content.

## Impact

- Applications using autoscraper in batch jobs / web services can hang indefinitely on one unresponsive host, tying up workers.
- Debugging is harder because a 404/block page looks identical to "no rules matched" (`[]`).

## Suggested Direction

- Add an explicit default, e.g. `request_args.setdefault("timeout", 10)` (or similar) in `_fetch_html` before calling `requests.get`, while still allowing caller override.
- After the GET, check `res.status_code` / call `res.raise_for_status()` (or raise a library-specific error with URL + status) so fetch failures are distinguishable from zero matches. Document the behavior in `build` / `get_result_*` docstrings.

## Evidence

- Source: `autoscraper/auto_scraper.py`, `AutoScraper._fetch_html` — the only `requests.get` call site in the package.
- Search of repo issues for `timeout` shows only incidental user-supplied `timeout` usage (e.g. issue #90), not a report of the missing default or missing status check, so this does not appear to be a duplicate.
- No `timeout`, `raise_for_status`, or `status_code` handling exists anywhere in `autoscraper/` per inspection of `auto_scraper.py` and `utils.py`.

## Classification

- FACT: `requests.get` is called without a default timeout and without status validation (verified in source via API).
- INFERENCE: this permits indefinite blocking and silent error-page parsing.
- HYPOTHESIS: a modest default timeout + status check would fix both without breaking the `request_args` escape hatch.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.