Skip to content

int(retry_after) raises ValueError on a non-integer Retry-After value #18

Description

@shachar-fastsimon

Summary

The 429 retry handlers in admin_graphql_request coerce Retry-After with int(), which raises ValueError on any value that is not a bare integer. HTTP permits a decimal seconds value as well as an HTTP-date, and decimal values are what Shopify's own rate-limited REST responses carry.

The exception is uncaught, so it propagates out of admin_graphql_request instead of being retried or returned as a GQLResult.

Version: shopifyapp 1.0.1 (sdist from PyPI).

Cause

shopify_app/graphql/admin_graphql.py:398 (sync):

time.sleep(int(retry_after))

and shopify_app/graphql/admin_graphql.py:454 (async):

await asyncio.sleep(int(retry_after))

Reproduction

>>> int("2.0")
Traceback (most recent call last):
  ...
ValueError: invalid literal for int() with base 10: '2.0'

Currently masked

This is not reachable today, because the header is never actually read: response_headers.get("Retry-After", "1") always returns its "1" default, for the reason described in #17.

That coupling is the reason this is worth filing on its own — repairing the header lookup alone turns a silent bug into an uncaught exception. The two are best addressed together.

Suggested fix

Parse defensively and fall back rather than raising:

try:
    delay = float(retry_after)
except (TypeError, ValueError):
    delay = 1.0
time.sleep(delay)

float() accepts the integer form as well, so it covers both spellings. If HTTP-date support is wanted, email.utils.parsedate_to_datetime handles that form.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    devtools-gardenerPost the issue or PR to Slack for the gardener

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions