requests icon indicating copy to clipboard operation
requests copied to clipboard

Preserve exception chain in morsel_to_cookie max-age conversion

Open tboy1337 opened this issue 3 months ago • 1 comments

Summary

This PR improves exception handling in the morsel_to_cookie function by preserving the original ValueError when a max-age conversion fails, following Python's exception chaining best practices.

Changes Made

  • Modified src/requests/cookies.py:

    • Updated the exception handling in morsel_to_cookie() to use raise ... from exc syntax
    • This preserves the original ValueError as the __cause__ attribute when raising a TypeError
  • Added test in tests/test_requests.py:

    • New test test_max_age_exception_chaining() to verify exception chaining behavior
    • Validates that the TypeError is raised with the correct message
    • Confirms the original ValueError is preserved in the exception chain
    • Verifies the ValueError message contains "invalid literal for int()"

Motivation

Previously, when an invalid max-age value was provided, the original ValueError from int() conversion was lost, making debugging more difficult. By using proper exception chaining (raise ... from exc), developers can now:

  • See the complete exception traceback
  • Understand both the high-level error (invalid max-age) and the low-level cause (ValueError from int conversion)
  • Leverage Python's exception chaining features for better debugging and logging

Testing

The new test ensures that:

  1. The appropriate TypeError is still raised for invalid max-age values
  2. The error message remains descriptive and helpful
  3. The original ValueError is accessible via the __cause__ attribute
  4. The complete exception chain is preserved for debugging purposes

Impact

  • No breaking changes - The external behavior remains the same (still raises TypeError)
  • Better debugging - Developers get more context when errors occur
  • Best practices - Follows PEP 3134 for explicit exception chaining

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