requests icon indicating copy to clipboard operation
requests copied to clipboard

Fix: Wrap unhandled urllib3 HTTPError types in RequestException

Open tboy1337 opened this issue 3 months ago • 1 comments

Summary

This PR enhances error handling in HTTPAdapter to properly wrap unhandled urllib3 HTTPError exceptions in RequestException, ensuring consistent exception handling across the requests library.

Problem

Previously, when urllib3 raised an HTTPError that wasn't explicitly handled (i.e., not SSLError, ReadTimeoutError, or InvalidHeader), the exception would propagate as-is. This created inconsistent error handling behavior where some urllib3 exceptions would bypass the requests exception hierarchy.

Solution

Modified src/requests/adapters.py:

  • Updated the exception handler in the send() method to wrap unknown HTTPError types in RequestException
  • Improved exception chaining by adding from err to all raised exceptions for better debugging and traceback clarity
  • Renamed the exception variable from e to err for improved code readability

Added comprehensive test coverage in tests/test_adapters.py:

  • Created test_unknown_httperror_wrapped_in_requestexception() to verify that generic urllib3 HTTPError exceptions are properly wrapped
  • Test confirms the exception is catchable as RequestException
  • Test verifies proper exception chaining via __cause__
  • Test validates the request object is properly attached to the exception

Changes

  • Files Modified: 2
  • Lines Added: 46
  • Lines Removed: 8

Technical Details

  • Bug Fix: Addresses unhandled urllib3 HTTPError types
  • Exception chaining: All exceptions now properly chain using from err
  • Backward compatible: No breaking changes to the public API

Testing

✅ New unit test added and passing ✅ Verifies proper exception wrapping behavior ✅ Confirms exception chaining integrity ✅ Validates request object attachment

tboy1337 avatar Oct 21 '25 13:10 tboy1337

I should add that this issue was found because of I think (I'm not 100% sure because everything was also typed according to mypy rules in the commit) pylint in https://github.com/psf/requests/pull/7054, if it was pylint that found this issue and it is a legitimate issue then it makes a strong case for the project to start using it imo.

tboy1337 avatar Oct 22 '25 12:10 tboy1337