Skip to content

fix: Retry on all impit.HTTPError exceptions, not just specific subclasses#624

Merged
vdusek merged 1 commit intomasterfrom
fix/retry-on-impit-http-error
Feb 18, 2026
Merged

fix: Retry on all impit.HTTPError exceptions, not just specific subclasses#624
vdusek merged 1 commit intomasterfrom
fix/retry-on-impit-http-error

Conversation

@vdusek
Copy link
Contributor

@vdusek vdusek commented Feb 17, 2026

Summary

  • Broadened is_retryable_error() to catch impit.HTTPError (base class) instead of only impit.NetworkError, impit.TimeoutException, and impit.RemoteProtocolError
  • Some transient errors (e.g. body decode failures like "unexpected EOF during chunk size line") are raised by impit as bare HTTPError instances that don't match any of the specific subclasses, causing requests to fail without retrying
  • HTTP status code errors are already handled separately in _make_request based on response status codes, so this change only affects transport-level failures

Triggered by: flaky e2e test failure in apify-sdk-python where wait_for_finish failed with:

impit.HTTPError: The internal HTTP library has thrown an error:
reqwest::Error {
    kind: Decode,
    source: reqwest::Error {
        kind: Body,
        source: hyper::Error(
            Body,
            Custom {
                kind: UnexpectedEof,
                error: "unexpected EOF during chunk size line",
            },
        ),
    },
}

Test plan

  • Added parametrized unit tests for is_retryable_error covering all impit exception types (retryable) and generic exceptions (non-retryable)
  • Added integration-style test test_retry_on_http_error_async_client that verifies bare HTTPError is actually retried by the HTTP client
  • All 115 unit tests pass
  • Pre-commit hooks pass (lint, type-check, docstrings)

🤖 Generated with Claude Code

…classes

Previously, `is_retryable_error` only retried `impit.NetworkError`,
`impit.TimeoutException`, and `impit.RemoteProtocolError`. However, some
transient errors (e.g. body decode failures like "unexpected EOF during
chunk size line") are raised as bare `impit.HTTPError` instances that
don't match any of those subclasses.

This caused flaky failures in downstream projects where `wait_for_finish`
calls would fail without retrying on transient network issues.

Broaden the check to `impit.HTTPError` which is the base class of all
impit transport-level exceptions. HTTP status code errors are already
handled separately in `_make_request` based on response status codes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added this to the 134th sprint - Tooling team milestone Feb 17, 2026
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Feb 17, 2026
@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.18%. Comparing base (e7ee8b3) to head (7d65893).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
+ Coverage   76.02%   76.18%   +0.16%     
==========================================
  Files          42       42              
  Lines        2482     2482              
==========================================
+ Hits         1887     1891       +4     
+ Misses        595      591       -4     
Flag Coverage Δ
integration 68.77% <ø> (ø)
unit 64.82% <ø> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek vdusek requested a review from barjin February 17, 2026 14:41
@vdusek vdusek added the bug Something isn't working. label Feb 17, 2026
@vdusek
Copy link
Contributor Author

vdusek commented Feb 17, 2026

@barjin Could you please confirm that all impit.HTTPError should be retried?

@vdusek vdusek added the adhoc Ad-hoc unplanned task added during the sprint. label Feb 17, 2026
@vdusek vdusek requested a review from Pijukatel February 17, 2026 15:39
Copy link
Member

@barjin barjin left a comment

Choose a reason for hiding this comment

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

impit is an HTTP client, all of its errors are HTTPError subclasses 😅 But yes, given the error typing might have some rough edges, this change is sound.

@vdusek vdusek merged commit 03b0e9c into master Feb 18, 2026
30 of 32 checks passed
@vdusek vdusek deleted the fix/retry-on-impit-http-error branch February 18, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. bug Something isn't working. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments