Skip to content

docs(storage): add gaxios and node 18 migration guide#8340

Open
thiyaguk09 wants to merge 61 commits into
googleapis:storage-node-18from
thiyaguk09:docs/gaxios-migration-guide
Open

docs(storage): add gaxios and node 18 migration guide#8340
thiyaguk09 wants to merge 61 commits into
googleapis:storage-node-18from
thiyaguk09:docs/gaxios-migration-guide

Conversation

@thiyaguk09

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 #<issue_number_goes_here> 🦕

…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label May 21, 2026

@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 refactors the library to version 8, bumping the minimum Node.js requirement to 18 and replacing the legacy request stack with a new StorageTransport layer based on gaxios and native Web APIs. The review identifies several critical issues in the new transport implementation, including a module-level global variable causing race conditions for project IDs, incorrect URL resolution logic, and broken interceptor support. Further feedback points out that per-object interceptors are no longer being applied, identifies fragile link extraction logic in GitHub scripts, and notes potential bugs related to unawaited async calls and synchronous project ID usage before it is fetched.

I am having trouble creating individual review comments. Click here to see my feedback.

handwritten/storage/src/storage-transport.ts (90)

critical

The use of a module-level global variable projectId to store state across requests is a critical bug. In a concurrent environment, multiple Storage instances or concurrent requests will overwrite this shared variable, leading to race conditions where a request for one project might use the ID of another. This state must be encapsulated within the StorageTransport instance.

handwritten/storage/src/storage-transport.ts (192)

critical

The URL construction logic uses string concatenation (${this.baseUrl}${pathUri}) instead of the native URL resolution rules. As noted in MIGRATION.md, a leading slash in pathUri should resolve relative to the host root, stripping segments like /storage/v1 from the baseUrl. The current implementation will incorrectly produce URLs with duplicated segments (e.g., .../storage/v1/storage/v1/...) because many paths in the library now include the full API prefix.

      url = new URL(pathUri, this.baseUrl);

handwritten/storage/conformance-test/conformanceCommon.ts (63)

high

This TODO effectively disables the methodMap initialization by using an empty object. This will cause conformance tests to skip all scenarios or fail silently, as the mapping between JSON test methods and library methods is lost.

  Object.entries(jsonToNodeApiMapping),

handwritten/storage/src/nodejs-common/service-object.ts (448-464)

high

The new implementation of getMetadata (and similarly setMetadata and delete) fails to pass this.interceptors to this.storageTransport.makeRequest. In previous versions, ServiceObject would merge parent and local interceptors. This change breaks the functionality of per-object interceptors (e.g., interceptors added specifically to a File or Bucket instance).

handwritten/storage/src/storage-transport.ts (134-139)

high

There are two issues here: 1) Clearing and adding interceptors on a shared this.gaxiosInstance is not thread-safe for concurrent requests. 2) The actual request is performed via this.authClient.request, which uses its own internal transporter and ignores the interceptors added to this.gaxiosInstance. This effectively breaks all interceptor support in the library.

handwritten/storage/.github/scripts/close-invalid-link.cjs (45)

medium

The logic for extracting the reproduction link is extremely fragile as it relies on a hardcoded line index (18). If a user adds extra lines or if the issue template is modified, this script will fail to find the link or pick up incorrect data. It is better to search the entire issue body for a GitHub/Gist URL following the expected section header.

handwritten/storage/src/bucket.ts (2081)

medium

If this.storage.projectId is not provided in the constructor and has not yet been fetched via an internal request (which triggers the async getProjectId in StorageTransport), this topic string will be incorrectly constructed as projects/undefined/topics/.... Since this is a synchronous assignment, it should account for the possibility of the project ID being unavailable.

handwritten/storage/src/file.ts (1478)

medium

The CopyCallback expects three arguments: (err, file, apiResponse). Calling it with only the error argument may cause issues for consumers expecting the full signature, especially if they are using promisified versions of the library that rely on specific argument positions.

      .catch(err => callback!(err, null, null));

handwritten/storage/src/resumable-upload.ts (1362)

medium

The call to attemptDelayedRetry is not awaited. Since attemptDelayedRetry is an async function that can call startUploading (also async), any rejection within that chain will result in an unhandled promise rejection because the caller (onResponse) has already returned.

      await this.attemptDelayedRetry(resp);

@thiyaguk09 thiyaguk09 marked this pull request as ready for review May 21, 2026 08:26
@thiyaguk09 thiyaguk09 requested a review from a team as a code owner May 21, 2026 08:26
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
@thiyaguk09 thiyaguk09 requested a review from a team as a code owner May 25, 2026 08:43
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
@shivanee-p shivanee-p requested review from a team and removed request for a team June 22, 2026 21:15
thiyaguk09 and others added 3 commits June 23, 2026 06:25
…sport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18
* fix(storage): standardize URL formatting and enhance transport retry

* fix storage transport & retry issues

* fix

* Update handwritten/storage/src/file.ts

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix(storage): interceptors test

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* feat: implement robust storage conformance test retry framework with request interception and test bench integration

* fix: correct response handler for binary/resumable uploads and improve etag check

- Updated `responseHandler` to correctly handle different payload types:
- Plain objects are mutated with `.headers` and `.status` and
returned.
- Binary payloads (Buffer/Stream) return raw data to prevent dangerous
mutations.
- Primitives (e.g., empty strings) return the full `GaxiosResponse`
wrapper to preserve access to headers like `Location` for resumable
upload initiation.
- Fixed `hasPrecondition` logic to safely parse stringified JSON or
inspect objects directly for an `etag` property. This prevents false
positives on raw text payloads containing the word "etag" and false
negatives on object payloads.

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* refactor(storage): replace constructor-based type checks with structural checks and decouple retry logic into idempotent and transient error utilities.

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* refactor(storage): update storage-transport to return full GaxiosResponse and align downstream resource methods

* fix: update file request URL construction to support custom protocol endpoints

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* refactor(storage): introduce ENCRYPTION_ALGORITHM_AES256 constant to replace hardcoded strings in File class

* fix(storage): merge request headers correctly in file.ts and add missing linting suppressions to ServiceObject

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* test: add bytes method to mock Gaxios responses in storage tests

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* test: add bytes method to mock Gaxios response in acl and headers tests

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* test: add bytes method to mock Gaxios response in acl and headers tests

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* test: add bytes method to mock Gaxios response in acl and headers tests

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* test: add bytes method to mock Gaxios response in acl and headers tests

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* fix(storage): standardize URL formatting and enhance transport retry

* refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283)

- Remove Service.ts and common.ts files from handwritten/storage

- Migrate remaining functionality to StorageTransport

- chore(ci): upgrade conformance tests to Node 18

* refactor: improve type safety and validation logic in isBucket helper function

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant