Uh oh!
There was an error while loading. Please reload this page.
fix(storage): log occasional (1 in 5 million) additional bytes received from GCS in read path - #17423
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces warning logs when more bytes are downloaded than requested from GCS, implemented in both _download.py and blob.py, along with a new unit test. The review feedback highlights several critical and medium-severity issues: an undefined headers variable in _download.py that will cause a NameError, a missing function call to _get_headers in blob.py that will fail at runtime, and a flawed unit test that does not actually execute the download loop due to incorrect mocking of download.finished and consume_next_chunk. Additionally, suggestions are provided to ensure fallback values are correctly applied when bucket or object names are None, and to align the range-handling logic in blob.py with _download.py for consistency.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
It has been found that GCS can occasionally send additional bytes while reading from stream. This scenario should be logged properly for debugging and tracking purposes. Fixes: 475824752 [Generated-by: AI]
d7a9e73 to
ad8a555Comparenidhiii-27
commented
Jun 11, 2026
Addressed all PR review comments. |
b09b205 to
d0dc047Compare| @@ -66,6 +66,8 @@ def __init__( | |||
| end=None, | |||
| headers=None, | |||
| retry=DEFAULT_RETRY, | |||
| client_info_bucket_name=None, | |||
There was a problem hiding this comment.
add these params in the doc string as well.
There was a problem hiding this comment.
Done
Co-authored by AI Agent
| if self.start is not None and self.start < 0 and self.end is None: | ||
| requested_length = -self.start | ||
| elif self.start is not None and self.end is not None: | ||
| requested_length = self.end - self.start + 1 | ||
| elif self.start is None and self.end is not None: | ||
| requested_length = self.end + 1 |
There was a problem hiding this comment.
this if else logic , can be made more readable a
ifstartisnotNoneandendisnotNone:
len=end-start+1elifstartisNone:
len=end+1elifstart<0:
len=-startand what about start > 0 ?
There was a problem hiding this comment.
Done. I have refactored the logic to make it cleaner and also handled the start > 0 with end is None case, which requests the remaining bytes from start to the end of the file.
Co-authored by AI Agent
[Generated-by: AI]
| @@ -66,6 +70,8 @@ def __init__( | |||
| end=None, | |||
| headers=None, | |||
| retry=DEFAULT_RETRY, | |||
| client_info_bucket_name=None, | |||
There was a problem hiding this comment.
you're introduced this new param , but no caller is passing these values. @nidhiii-27
There was a problem hiding this comment.
Good point. I have reverted all changes (reverted the warning logging and the unused client_info parameters) in _download.py and consolidated the warning logging logic inside blob.py after the download loop. I also expanded the requested_length calculation in blob.py to handle all range request cases correctly.
Co-authored by AI Agent
There was a problem hiding this comment.
instead of doing changes in this files, why not after these lines -
google-cloud-python/packages/google-cloud-storage/google/cloud/storage/blob.py
Lines 1123 to 1124 in 8cb77d9
you don't have to pass bucket_name and object_name ?
There was a problem hiding this comment.
Good point. I have reverted all changes (reverted the warning logging and the unused client_info parameters) in _download.py and consolidated the warning logging logic inside blob.py after the download loop. I also expanded the requested_length calculation in blob.py to handle all range request cases correctly.
Co-authored by AI Agent
…nload.py to blob.py [Generated-by: AI]
…c readability [Generated-by: AI]
…in blob.py [Generated-by: AI]
| response = download.consume_next_chunk(transport, timeout=timeout) | ||
| requested_length = None | ||
| if start is None: | ||
| if end is None: | ||
| total_bytes = getattr(download, "total_bytes", None) | ||
| if isinstance(total_bytes, (int, float)): | ||
| requested_length = total_bytes | ||
| else: | ||
| requested_length = end + 1 | ||
| else: | ||
| if end is not None: | ||
| requested_length = end - start + 1 | ||
| else: | ||
| if start < 0: | ||
| requested_length = -start | ||
| else: | ||
| total_bytes = getattr(download, "total_bytes", None) | ||
| if isinstance(total_bytes, (int, float)): | ||
| requested_length = total_bytes - start | ||
| if isinstance(requested_length, (int, float)) and requested_length >= 0: | ||
| received_bytes = getattr(download, "_bytes_downloaded", 0) | ||
| if isinstance(received_bytes, int) and received_bytes > requested_length: | ||
| from google.cloud.storage._media import _helpers as media_helpers | ||
| if ( | ||
| response is not None | ||
| and not media_helpers._is_decompressive_transcoding( | ||
| response, download._get_headers | ||
| ) | ||
| ): | ||
| _logger.warning( | ||
| "storage: received %d more bytes than requested from GCS for bucket %r, object %r", | ||
| received_bytes - requested_length, | ||
| self.bucket.name, | ||
| self.name, | ||
| ) |
There was a problem hiding this comment.
Since the issue b/475824752 is valid only for range requests where end_byte < last_byte of the object.
hence I think we can further simplify this logic.
if end is not None:
if start is None:
start = 0
requested_len = end - start + 1
if download._bytes_downloaded > requested_len:
_logger.warning(" extra bytes downloaded") There was a problem hiding this comment.
Done
Co-authored by AI Agent
Simplify the range request warning logic in blob.py to only trigger when end is not None, resolving review feedback. [Generated-by: AI]
Uh oh!
There was an error while loading. Please reload this page.
🤖 I have created a release *beep* *boop* --- <details><summary>db-dtypes: 1.7.1</summary> ## [1.7.1](db-dtypes-v1.7.0...db-dtypes-v1.7.1) (2026-07-07) ### Bug Fixes * avoid deprecated unitless operations for NumPy 2.5 compatibility ([#17589](#17589)) ([d0b2abc](d0b2abc)) </details> <details><summary>gapic-generator: 1.37.0</summary> ## [1.37.0](gapic-generator-v1.36.0...gapic-generator-v1.37.0) (2026-07-07) ### Features * implement native PEP 0810 lazy loading ([#17591](#17591)) ([8a1270c](8a1270c)) ### Bug Fixes * **deps:** bump google-api-core to 2.25.0 ([#17599](#17599)) ([8b359e2](8b359e2)) * **tests:** add --cov-append to gapic-generator and proto-plus to preserve monorepo coverage ([#17603](#17603)) ([2ddcf4d](2ddcf4d)) </details> <details><summary>google-auth: 2.55.2</summary> ## [2.55.2](google-auth-v2.55.1...google-auth-v2.55.2) (2026-07-07) ### Bug Fixes * **auth:** Agentic Identites mTLS gaps fix _is_mtls and SslCredentials. ([#17387](#17387)) ([7bfa41a](7bfa41a)) * **auth:** align mTLS discovery and enforce fail-fast transport configuration. ([#17470](#17470)) ([f492d3d](f492d3d)) * **auth:** handle PermissionError on workload certificates to avoid startup hang and crash ([#17568](#17568)) ([f538ad8](f538ad8)) </details> <details><summary>google-cloud-agentregistry: 0.1.0</summary> ## 0.1.0 (2026-07-07) ### Features * **google/cloud/agentregistry/v1:** add google-cloud-agentregistry ([#17565](#17565)) ([f479800](f479800)) </details> <details><summary>google-cloud-biglake-hive: 0.3.1</summary> ## [0.3.1](google-cloud-biglake-hive-v0.3.0...google-cloud-biglake-hive-v0.3.1) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-bigquery: 3.42.2</summary> ## [3.42.2](google-cloud-bigquery-v3.42.1...google-cloud-bigquery-v3.42.2) (2026-07-07) ### Bug Fixes * **bigquery:** avoid SSLError retry loop ([#17489](#17489)) ([8248d8e](8248d8e)) * include amended user agent in read client ([#17592](#17592)) ([c43caee](c43caee)) </details> <details><summary>google-cloud-binary-authorization: 1.18.0</summary> ## [1.18.0](google-cloud-binary-authorization-v1.17.0...google-cloud-binary-authorization-v1.18.0) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-chronicle: 0.6.2</summary> ## [0.6.2](google-cloud-chronicle-v0.6.1...google-cloud-chronicle-v0.6.2) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-dataform: 0.11.2</summary> ## [0.11.2](google-cloud-dataform-v0.11.1...google-cloud-dataform-v0.11.2) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-dataproc: 5.30.0</summary> ## [5.30.0](google-cloud-dataproc-v5.29.0...google-cloud-dataproc-v5.30.0) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-datastore: 2.26.0</summary> ## [2.26.0](google-cloud-datastore-v2.25.0...google-cloud-datastore-v2.26.0) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-dialogflow: 2.50.0</summary> ## [2.50.0](google-cloud-dialogflow-v2.49.0...google-cloud-dialogflow-v2.50.0) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-kms: 3.15.0</summary> ## [3.15.0](google-cloud-kms-v3.14.0...google-cloud-kms-v3.15.0) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-logging: 3.16.1</summary> ## [3.16.1](google-cloud-logging-v3.16.0...google-cloud-logging-v3.16.1) (2026-07-07) ### Documentation * **logging:** fix StructuredLogHandler docstring parameter name ([#17625](#17625)) ([32862f0](32862f0)), closes [#17604](#17604) </details> <details><summary>google-cloud-modelarmor: 0.7.1</summary> ## [0.7.1](google-cloud-modelarmor-v0.7.0...google-cloud-modelarmor-v0.7.1) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-network-security: 0.13.3</summary> ## [0.13.3](google-cloud-network-security-v0.13.2...google-cloud-network-security-v0.13.3) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-saasplatform-saasservicemgmt: 0.7.1</summary> ## [0.7.1](google-cloud-saasplatform-saasservicemgmt-v0.7.0...google-cloud-saasplatform-saasservicemgmt-v0.7.1) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-securesourcemanager: 0.6.1</summary> ## [0.6.1](google-cloud-securesourcemanager-v0.6.0...google-cloud-securesourcemanager-v0.6.1) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>google-cloud-storage: 3.12.1</summary> ## [3.12.1](google-cloud-storage-v3.12.0...google-cloud-storage-v3.12.1) (2026-07-07) ### Bug Fixes * **storage:** log occasional (1 in 5 million) additional bytes received from GCS in read path ([#17423](#17423)) ([335c12f](335c12f)) </details> <details><summary>google-cloud-support: 0.5.1</summary> ## [0.5.1](google-cloud-support-v0.5.0...google-cloud-support-v0.5.1) (2026-07-07) ### Features * update googleapis and regenerate ([#17635](#17635)) ([9638879](9638879)) </details> <details><summary>proto-plus: 1.28.1</summary> ## [1.28.1](proto-plus-v1.28.0...proto-plus-v1.28.1) (2026-07-07) ### Bug Fixes * **tests:** add --cov-append to gapic-generator and proto-plus to preserve monorepo coverage ([#17603](#17603)) ([2ddcf4d](2ddcf4d)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
It has been found that GCS can occasionally ( 1 in 5 million) send additional bytes while reading from stream. This scenario should be logged properly for debugging and tracking purposes.
Fixes: 475824752