Skip to content

fix(gax): propagate chunk granularity on resume and preserve retry error details - #9480

Merged
feywind merged 4 commits into
googleapis:mainfrom
feywind:resumable/gax-scotty-fixes
Oct 1, 2026
Merged

feywind merged 4 commits into
googleapis:mainfrom
feywind:resumable/gax-scotty-fixes

Conversation

@feywind

@feywind feywind commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Improves ResumableUploadSession handling of server-specified chunk granularity during session start, recovery queries, and resumption, and preserves underlying failure details when retries are exhausted:

  1. Chunk granularity on recovery & resume (x-goog-upload-chunk-granularity):
    • Parse x-goog-upload-chunk-granularity in queryOffset() and propagate it when resuming via resumeUrl, reconciling start responses, or recovering mid-transfer.
    • Compute effectiveChunkSize_ before invoking reportProgress() during start() so session.chunkSize reflects the negotiated chunk size inside onProgress callbacks from the very first progress event.
    • If a mid-transfer recovery query negotiates a different effectiveChunkSize_ than the non-final chunk currently in flight, restart transmission from serverOffset with the aligned chunk size.
  2. Retry start when both x-goog-upload-status and x-goog-upload-url are missing:
    • When a 2xx start response omits x-goog-upload-status and has no x-goog-upload-url to reconcile via query, classify it as a TransientError so start retries with backoff.
  3. Preserve underlying failure details when retries are exhausted:
    • Append the underlying error message when sendCommandWithRetry, transmitChunk, or transmitFinalize exhausts maxRetries, and avoid double-wrapping TransientError in fetchCommand.
  4. Tests:
    • Added unit tests, system test coverage, and a gapic-showcase chunk_granularity pause-and-resume test scenario.

@feywind
feywind requested a review from a team as a code owner September 30, 2026 17:29

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

Copy link
Copy Markdown
Contributor

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 enhances the ResumableUploadSession to support chunk granularity during resumable uploads, ensuring that chunk sizes are properly aligned with the server's requirements during recovery queries and session starts. It also improves error handling by propagating underlying error messages when retries are exhausted and throwing a TransientError when start headers are missing. The feedback suggests defensively checking the type of the caught error before accessing its .message property to prevent potential runtime TypeErrors when formatting error messages.

Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/src/resumableUpload.ts
@github-actions
github-actions Bot requested a review from shivanee-p September 30, 2026 17:50

@quirogas quirogas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the changes and ran local empirical tests against the new granularity propagation and retry error handling paths. Left a few inline comments on edge cases we reproduced.

Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/src/resumableUpload.ts
Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/src/resumableUpload.ts Outdated
Comment thread core/packages/gax/test/unit/resumableUpload.ts
Comment thread core/packages/gax/test/system-test/resumableUpload.ts Outdated
@quirogas
quirogas self-requested a review October 1, 2026 20:52
@feywind
feywind merged commit 02b9478 into googleapis:main Oct 1, 2026
46 checks passed
@release-please release-please Bot mentioned this pull request Oct 1, 2026
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