Skip to content

refactor(storage): make onResponse async and improve retry error handling#8753

Open
thiyaguk09 wants to merge 7 commits into
googleapis:mainfrom
thiyaguk09:fix/resumable-upload-error-serialization
Open

refactor(storage): make onResponse async and improve retry error handling#8753
thiyaguk09 wants to merge 7 commits into
googleapis:mainfrom
thiyaguk09:fix/resumable-upload-error-serialization

Conversation

@thiyaguk09

@thiyaguk09 thiyaguk09 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #7346

- Unify formatting of Errors and objects under a single block inside
formatRetryError to reduce code duplication.
- Ensure standard errors, GaxiosErrors, and custom errors with
empty/missing properties are correctly formatted.

@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 introduces a helper function buildRetryError to format detailed error messages when retries are exceeded, and updates the retry logic and tests accordingly. However, the changes also make onResponse and attemptDelayedRetry asynchronous. The reviewer correctly points out that these functions do not perform any asynchronous operations, making the async and await keywords unnecessary. Feedback is provided to revert these functions and their associated tests back to their synchronous implementations to avoid unnecessary overhead.

Comment thread handwritten/storage/src/resumable-upload.ts Outdated
Comment thread handwritten/storage/src/resumable-upload.ts Outdated
Comment thread handwritten/storage/src/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
Comment thread handwritten/storage/test/resumable-upload.ts Outdated
@thiyaguk09 thiyaguk09 marked this pull request as ready for review June 25, 2026 12:47
@thiyaguk09 thiyaguk09 requested a review from a team as a code owner June 25, 2026 12:47
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.

How do I get more information about failing uploads?

1 participant