Skip to content

Allow configuring open and read timeouts for API requests - #1467

Open
fluke wants to merge 2 commits into
Shopify:mainfrom
fluke:http-timeouts
Open

fluke wants to merge 2 commits into
Shopify:mainfrom
fluke:http-timeouts

Conversation

@fluke

@fluke fluke commented Oct 4, 2026

Copy link
Copy Markdown

Description

Fixes #1466

  • Problem: HttpClient#request calls HTTParty with no timeout, so every API request uses Net::HTTP's default 60-second read timeout. When Shopify is slow to respond, the calling thread (often a web request worker) waits for the whole stall. In production we saw an Admin GraphQL read hold a Puma thread for 27.5s.
  • Change: adds optional open_timeout and read_timeout (seconds) to ShopifyAPI::Context.setup. HttpClient#request passes the ones that are set to HTTParty.
  • Impact: nothing changes unless an app sets them. A timed-out request raises Net::OpenTimeout / Net::ReadTimeout. The retry loop only retries 429/500 responses, so timeouts aren't retried; the README says so.

This is deliberately narrower than #1376, which exposes a general httparty_params hash. Only the two timeouts are added, as typed settings. If you'd rather take #1376, it covers this as well.

How has this been tested?

  • test/clients/http_client_test.rb:
    • timeouts set on Context reach the Net::HTTP connection;
    • with none set, the connection's timeouts aren't touched;
    • a Net::ReadTimeout is raised and not retried.
  • test/context_test.rb: both settings default to nil and can be configured.
  • bundle exec rake test:library: 347 runs, 0 failures. rubocop and srb tc are clean.
  • The first http_client_test test fails when **timeouts is removed from the HTTParty call.

Checklist:

  • My commit message follow the pattern described in here
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the project documentation.
  • I have added a changelog line.

🤖 Generated with Claude Code

fluke and others added 2 commits October 4, 2026 13:20
HttpClient#request calls HTTParty without a timeout, so a stalled
response holds the calling thread for Net::HTTP's default 60s read
timeout. Add optional open_timeout and read_timeout to Context.setup and
pass them to HTTParty on every request. Unset, behaviour is unchanged.

Fixes Shopify#1466

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the devtools-gardener Post the issue or PR to Slack for the gardener label Oct 4, 2026
@fluke

fluke commented Oct 4, 2026

Copy link
Copy Markdown
Author

I have signed the CLA!

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow configuring open/read timeouts for API requests

1 participant