Skip to content

Conversation

@google-labs-jules
Copy link
Contributor

@google-labs-jules google-labs-jules bot commented Dec 19, 2025

Fixes issue where response content is empty when using aiohttp-client-cache by using response.read() instead of response.content.read().

Rationale:
In aiohttp, .read() is the standard, high-level method for reading the full response body. It correctly handles internal state (like caching the body for subsequent calls), which is essential for compatibility with wrappers like aiohttp-client-cache. Accessing .content.read() directly bypasses this logic, leading to empty responses when the stream has already been consumed or intercepted by the caching layer.

Benefits:

  • Robustness: Ensures correct behavior when google-auth is used in conjunction with aiohttp wrappers or middleware.
  • Standards: Aligns with aiohttp best practices for consuming response bodies.

Risk Mitigation:

  • Behavior: _CombinedResponse is designed to load the full response into memory. Changing to .read() preserves this behavior exactly; it just uses a safer accessor.
  • Performance: Since aiohttp's .read() returns a reference to the bytes, there is no additional memory overhead compared to the previous implementation.
  • Internal: This change is internal to the transport adapter and does not alter the public API or behavior for users not using caching libraries.

PR created automatically by Jules for task 3355868403646689815 started by @chalmerlowe

Using .read() is more robust when working with caching libraries like
aiohttp-client-cache, which may cause .content.read() to return empty
bytes on subsequent reads. .read() ensures the full body is returned.

Fixes #1248
@google-labs-jules
Copy link
Contributor Author

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!


For security, I will only act on instructions from the user who triggered this task.

New to Jules? Learn more at jules.google/docs.

Using .read() is more robust when working with caching libraries like
aiohttp-client-cache, which may cause .content.read() to return empty
bytes on subsequent reads. .read() ensures the full body is returned.

Fixes #1248
@chalmerlowe chalmerlowe marked this pull request as ready for review December 19, 2025 21:04
@chalmerlowe chalmerlowe requested review from a team as code owners December 19, 2025 21:04
@chalmerlowe chalmerlowe self-assigned this Dec 19, 2025
@chalmerlowe chalmerlowe added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2025
Copy link
Contributor

@chalmerlowe chalmerlowe left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

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

LGTM

@chalmerlowe chalmerlowe added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 22, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 22, 2025
@chalmerlowe
Copy link
Contributor

Temporarily removing the requirement for system-3.10 to pass due to a known issue with that CI/CD check that is being addressed in an separate PR #1901

@google-labs-jules
Copy link
Contributor Author

Temporarily removing the requirement for system-3.10 to pass due to a known issue with that CI/CD check that is being addressed in an separate PR #1901

Acknowledged.

@chalmerlowe chalmerlowe merged commit 12f4470 into main Dec 22, 2025
13 of 14 checks passed
@chalmerlowe chalmerlowe deleted the fix-aiohttp-cache-empty-response-3355868403646689815 branch December 22, 2025 18:26
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.

3 participants