Skip to content

fix: add request timeout to paginated fetch helpers - #2477

Open
bunlongheng wants to merge 1 commit into
wger-project:masterfrom
bunlongheng:fix-requests-missing-timeout-sec
Open

bunlongheng wants to merge 1 commit into
wger-project:masterfrom
bunlongheng:fix-requests-missing-timeout-sec

Conversation

@bunlongheng

Copy link
Copy Markdown

What

get_all_paginated() and get_paginated() in wger/utils/requests.py call requests.get() without a timeout. These two helpers back most of the remote-sync code paths (sync_exercises, sync_languages, sync_licenses, sync_categories, sync_muscles, sync_equipment, handle_deleted_entries, download_exercise_images, download_exercise_videos, download_ingredient_images, sync_ingredients, ...).

Why it matters

Without a timeout, requests blocks indefinitely when a remote server accepts the connection but never sends a response (a stalled or misbehaving upstream, a hung reverse proxy, a slow-loris style peer). The affected calls run both from management commands and from celery tasks (e.g. fetch_all_ingredient_images_task -> download_ingredient_images). A single hung request pins the worker/process forever - it never returns and never raises, so the autoretry_for=(requests.exceptions.RequestException, ...) handling on the surrounding tasks can never kick in either.

This is the same defensive pattern the codebase already applies at newer call sites, which pass explicit timeouts (requests.get(..., timeout=30) in sync_ingredients, timeout=(10, 60) in download_ingredient_dump). These two central helpers were simply missed.

Fix

Add a module-level DEFAULT_REQUEST_TIMEOUT = (10, 60) (connect, read) constant and pass it to both requests.get() calls. The tuple form matches the existing style in nutrition/sync.py. No signature or behavior change for successful responses.

Test

  • sync_exercises / sync_ingredients against a reachable remote continue to work unchanged.
  • Point a sync at a host that accepts the TCP connection but never responds (e.g. firewall DROP on an open port); the call now raises requests.exceptions.ReadTimeout after 60s instead of hanging forever, allowing the caller's retry/error handling to run.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants