Skip to content

Conversation

patchback[bot]
Copy link
Contributor

@patchback patchback bot commented Jul 15, 2025

This is a backport of PR #11301 as merged into master (815901f).

What do these changes do?

This PR addresses review comments from PR #11290 by:

  • Removing redundant inner try-except block in IOBasePayload.size property - the exceptions are already handled by the outer try-except
  • Updating stale test comments that referenced the bug as if it was still present
  • Improving test docstrings to reflect that they verify the fix rather than demonstrate a bug

Are there changes in behavior for the user?

No behavioral changes. This is purely a code cleanup that removes redundancy while maintaining the exact same functionality.

Is it a substantial burden for the maintainers to support this?

No, this actually reduces maintenance burden by:

  • Removing redundant code that could cause confusion
  • Making the code flow clearer and easier to understand
  • Ensuring test documentation accurately reflects the current state

Related issue number

Follow-up to #11290 (addressing review comments)
Related to #11270 (original bug fix)

Checklist

  • I think the code is well written
  • Unit tests for the changes exist (all existing tests pass)
  • Documentation reflects the changes (no user-facing docs needed)
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES/ folder

Summary of Changes

1. Removed redundant code in aiohttp/payload.py:

# Before:
if self._start_position is None:
    try:
        self._start_position = self._value.tell()
    except (OSError, AttributeError):
        # Can't get position, can't determine size
        return None

# After:
if self._start_position is None:
    self._start_position = self._value.tell()

The inner try-except was redundant because:

  • The outer try-except block in the size property already catches AttributeError and OSError
  • Any exception raised by self._value.tell() will be caught by the outer handler
  • Having nested try-except blocks for the same exceptions adds unnecessary complexity

2. Updated test comments in tests/test_payload.py:

  • Removed comments claiming "This assertion fails!" and "This also fails!" from test_iobase_payload_size_after_reading
  • Updated docstring from "demonstrates the bug" to "verifies that size calculation properly accounts for the initial file position"

3. Updated test comments in tests/test_client_functional.py:

  • Updated docstring in test_file_upload_307_308_redirect from "demonstrates the bug" to "verifies that file payloads maintain correct Content-Length"

All tests continue to pass, confirming the redundant code was indeed unnecessary.

Copy link

codspeed-hq bot commented Jul 15, 2025

CodSpeed Performance Report

Merging #11318 will not alter performance

Comparing patchback/backports/3.12/815901f6afb2c460650ccb0b6f0bfa5b55fe5cd9/pr-11301 (2789b70) with 3.12 (36de40a)

Summary

✅ 59 untouched benchmarks

Copy link

codecov bot commented Jul 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.23%. Comparing base (36de40a) to head (2789b70).

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             3.12   #11318      +/-   ##
==========================================
- Coverage   98.32%   98.23%   -0.09%     
==========================================
  Files         132      130       -2     
  Lines       43679    43611      -68     
  Branches     2384     2373      -11     
==========================================
- Hits        42946    42840     -106     
- Misses        558      595      +37     
- Partials      175      176       +1     
Flag Coverage Δ
CI-GHA 98.12% <100.00%> (-0.09%) ⬇️
OS-Linux 97.88% <100.00%> (-0.09%) ⬇️
OS-Windows 95.67% <100.00%> (-0.01%) ⬇️
OS-macOS 97.22% <100.00%> (-0.01%) ⬇️
Py-3.10.11 96.86% <100.00%> (+<0.01%) ⬆️
Py-3.10.18 97.26% <100.00%> (-0.09%) ⬇️
Py-3.11.13 97.45% <100.00%> (-0.09%) ⬇️
Py-3.11.9 97.05% <100.00%> (-0.01%) ⬇️
Py-3.12.10 97.14% <100.00%> (-0.01%) ⬇️
Py-3.12.11 97.54% <100.00%> (-0.09%) ⬇️
Py-3.13.3 97.78% <100.00%> (-0.10%) ⬇️
Py-3.9.13 96.75% <100.00%> (-0.01%) ⬇️
Py-3.9.23 97.16% <100.00%> (-0.08%) ⬇️
Py-pypy7.3.16 85.45% <100.00%> (-5.07%) ⬇️
VM-macos 97.22% <100.00%> (-0.01%) ⬇️
VM-ubuntu 97.88% <100.00%> (-0.09%) ⬇️
VM-windows 95.67% <100.00%> (-0.01%) ⬇️

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.

@Dreamsorcerer Dreamsorcerer merged commit 3ff61ce into 3.12 Jul 15, 2025
36 checks passed
@Dreamsorcerer Dreamsorcerer deleted the patchback/backports/3.12/815901f6afb2c460650ccb0b6f0bfa5b55fe5cd9/pr-11301 branch July 15, 2025 12:24
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.

2 participants