Skip to content

Conversation

tboy1337
Copy link

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

…riginal ValueError during max-age conversion failure. Added a test to verify exception chaining behavior.
@tboy1337
Copy link
Author

tboy1337 commented Oct 22, 2025

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 #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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant