Skip to content

fix(storage): fix panic error in transfermanager downloads#13815

Merged
krishnamd-jkp merged 4 commits intogoogleapis:mainfrom
krishnamd-jkp:fix_transfer_manager_error
Feb 9, 2026
Merged

fix(storage): fix panic error in transfermanager downloads#13815
krishnamd-jkp merged 4 commits intogoogleapis:mainfrom
krishnamd-jkp:fix_transfer_manager_error

Conversation

@krishnamd-jkp
Copy link
Contributor

@krishnamd-jkp krishnamd-jkp commented Feb 9, 2026

No description provided.

@krishnamd-jkp krishnamd-jkp requested a review from a team February 9, 2026 08:51
@krishnamd-jkp krishnamd-jkp requested review from a team as code owners February 9, 2026 08:51
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Feb 9, 2026
@gemini-code-assist
Copy link
Contributor

gemini-code-assist bot commented Feb 9, 2026

Summary of Changes

Hello @krishnamd-jkp, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a critical fix to prevent a panic error that could occur during transfer manager downloads, specifically within the gatherShards function. The core change involves refining the error handling logic to ensure that the orderedChecksums array is only accessed with valid shard indices, thereby preventing out-of-bounds access. Complementing this fix, the associated unit tests have been thoroughly refactored to provide more robust and clear validation for various download scenarios, including error conditions and edge cases.

Highlights

  • Panic Prevention: Addressed a potential panic in the gatherShards function within the transfer manager. The panic could occur when processing DownloadOutput objects with a shard value of 0 or after an error, by attempting to access orderedChecksums with an invalid index.
  • Improved Error Handling Logic: Enhanced the gatherShards function to immediately continue the loop after an error is encountered and the context is cancelled. This prevents further processing of potentially invalid shardOut data and ensures orderedChecksums is only updated for valid, positive shard indices.
  • Refactored Test Suite: The TestGatherShards unit test has been completely refactored into a comprehensive table-driven test. This significantly improves test coverage and clarity for various scenarios, including successful downloads, shard failures, checksum mismatches, and the specific edge case of an invalid shard index (0).
Changelog
  • storage/transfermanager/downloader.go
    • Added a continue statement within the gatherShards function to skip further processing of a shard after an error is detected and the context is cancelled, preventing potential panics.
    • Introduced a conditional check if shardOut.shard > 0 before updating orderedChecksums to ensure that array access is always performed with a valid, positive index.
  • storage/transfermanager/downloader_test.go
    • Completely refactored the TestGatherShards function to utilize a table-driven test approach, improving test organization and readability.
    • Expanded test cases to cover successful shard gathering, scenarios where individual shards fail, checksum mismatches, and the specific case of an invalid shard index (0), directly addressing the panic fix.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 9, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively resolves a panic in the transfer manager's downloader which could occur when a download shard failed. The fix is well-implemented, preventing an out-of-bounds slice access by adding a continue statement in the error handling path and a guard for the shard index. The accompanying test for gatherShards has been substantially improved through refactoring into a table-driven test, which enhances readability and coverage for various scenarios, including the one that triggered the panic. My feedback includes a suggestion to further enhance the new test by re-introducing a check for context cancellation on shard failure, a behavior that was verified in the previous version of the test. This comment aligns with existing practices and does not contradict any provided rules.

@krishnamd-jkp
Copy link
Contributor Author

krishnamd-jkp commented Feb 9, 2026

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 9, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses a critical panic error in the transfer manager's download process by ensuring that checksums are not processed for errored shards and by adding a defensive check for valid shard indices. The accompanying test suite has been significantly improved, transitioning to a robust table-driven approach that covers various success and failure scenarios, including the specific panic condition. This enhances both the correctness and reliability of the downloader.

cpriti-os
cpriti-os requested changes Feb 9, 2026
View reviewed changes
@krishnamd-jkp krishnamd-jkp merged commit 4272271 into googleapis:main Feb 9, 2026
10 checks passed
krishnamd-jkp added a commit that referenced this pull request Feb 9, 2026
@krishnamd-jkp
fix(storage): fix panic error in transfermanager downloads (#13815)
48876af
This was referenced Feb 9, 2026
Closed
Merged
krishnamd-jkp added a commit that referenced this pull request Feb 10, 2026
@krishnamd-jkp
chore: librarian release pull request: 20260210T090937Z (#13824)
7783bb0 
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v0.8.0
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-go@sha256:19bb93e8f1f916c61b597db2bad65dc432f79baaabb210499d7d0e4ad1dffe29
<details><summary>storage: 1.60.0</summary>

##
[1.60.0](storage/v1.59.1...storage/v1.60.0)
(2026-02-10)

### Features

* adding support for max-retry-duration for all api other than
resumable-upload (#13749)
([31c352b](31c352bd))

* support checksum validation in resumable json uploads (#13573)
([337ca07](337ca078))

* Added a new field `ComposeObjectRequest.delete_source_objects` field
(PiperOrigin-RevId: 863087065)
([611f239](611f2392))

### Bug Fixes

* change TestValidateChecksumFromServer error message (#13810)
([0741414](07414143))

* fix panic error in transfermanager downloads (#13815)
([4272271](42722719))

* deadlock in event loop while coordinating channels (#13652)
([7d9d00c](7d9d00c6))

### Documentation

* Updated documentation for `BidiReadObject`, `ReadObjectRequest`, and
`ObjectContexts` (PiperOrigin-RevId: 863087065)
([611f239](611f2392))

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

Reviewers

@cpriti-os cpriti-os cpriti-os approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@krishnamd-jkp @cpriti-os