Skip to content

Commit b16d16a

Browse files
fix(storage): Invocation ID is not retained on multipart upload retries (googleapis#8190)
Hoists the generation of `persistentInvocationId` to the beginning of the upload process in `Bucket.upload` and `File.save`. This ensures that retried multipart upload attempts reuse the same invocation ID in the `x-goog-api-client` header, rather than generating a new one for each attempt. 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](https://github.com/googleapis/{{metadata['repo']['name']}}/issues) 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> 🦕 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 58ef9bf commit b16d16a

7 files changed

Lines changed: 454 additions & 26 deletions

File tree

handwritten/storage/src/bucket.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import * as http from 'http';
2929
import * as path from 'path';
3030
import {promisify} from 'util';
3131
import AsyncRetry from 'async-retry';
32+
import {randomUUID} from 'crypto';
3233
import {convertObjKeysToSnakeCase, handleContextValidation} from './util.js';
3334

3435
import {Acl, AclMetadata} from './acl.js';
@@ -38,6 +39,7 @@ import {
3839
FileOptions,
3940
CreateResumableUploadOptions,
4041
CreateWriteStreamOptions,
42+
CreateWriteStreamOptionsInternal,
4143
FileMetadata,
4244
ContextValue,
4345
} from './file.js';
@@ -4511,6 +4513,7 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
45114513
optionsOrCallback?: UploadOptions | UploadCallback,
45124514
callback?: UploadCallback,
45134515
): Promise<UploadResponse> | void {
4516+
const persistentInvocationId = randomUUID();
45144517
const upload = (numberOfRetries: number | undefined) => {
45154518
const returnValue = AsyncRetry(
45164519
async (bail: (err: GaxiosError | Error) => void) => {
@@ -4521,7 +4524,10 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
45214524
) {
45224525
newFile.storage.retryOptions.autoRetry = false;
45234526
}
4524-
const writable = newFile.createWriteStream(options);
4527+
const writable = newFile.createWriteStream({
4528+
...options,
4529+
invocationId: persistentInvocationId,
4530+
} as CreateWriteStreamOptionsInternal);
45254531
if (options.onUploadProgress) {
45264532
writable.on('progress', options.onUploadProgress);
45274533
}

handwritten/storage/src/file.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import * as resumableUpload from './resumable-upload.js';
2727
import {Writable, Readable, pipeline, Transform, PipelineSource} from 'stream';
2828
import * as zlib from 'zlib';
2929
import * as http from 'http';
30+
import {randomUUID} from 'crypto';
3031

3132
import {
3233
ExceptionMessages,
@@ -266,6 +267,14 @@ export interface CreateWriteStreamOptions extends CreateResumableUploadOptions {
266267
validation?: string | boolean;
267268
}
268269

270+
/**
271+
* @internal
272+
*/
273+
export interface CreateWriteStreamOptionsInternal
274+
extends CreateWriteStreamOptions {
275+
invocationId?: string;
276+
}
277+
269278
export interface MakeFilePrivateOptions {
270279
metadata?: FileMetadata;
271280
strict?: boolean;
@@ -2212,7 +2221,10 @@ class File extends ServiceObject<File, FileMetadata> {
22122221

22132222
writeStream.once('writing', async () => {
22142223
if (options.resumable === false) {
2215-
await this.startSimpleUpload_(fileWriteStream, options);
2224+
await this.startSimpleUpload_(
2225+
fileWriteStream,
2226+
options as CreateWriteStreamOptionsInternal,
2227+
);
22162228
} else {
22172229
await this.startResumableUpload_(fileWriteStream, options);
22182230
}
@@ -4218,13 +4230,17 @@ class File extends ServiceObject<File, FileMetadata> {
42184230
) {
42194231
maxRetries = 0;
42204232
}
4233+
const persistentInvocationId = randomUUID();
42214234
const returnValue = AsyncRetry(
42224235
async (bail: (err: Error) => void) => {
42234236
return new Promise<void>((resolve, reject) => {
42244237
if (maxRetries === 0) {
42254238
this.storage.retryOptions.autoRetry = false;
42264239
}
4227-
const writable = this.createWriteStream(options);
4240+
const writable = this.createWriteStream({
4241+
...options,
4242+
invocationId: persistentInvocationId,
4243+
} as CreateWriteStreamOptionsInternal);
42284244

42294245
if (options.onUploadProgress) {
42304246
writable.on('progress', options.onUploadProgress);
@@ -4531,7 +4547,7 @@ class File extends ServiceObject<File, FileMetadata> {
45314547
*/
45324548
startSimpleUpload_(
45334549
dup: Duplexify,
4534-
options: CreateWriteStreamOptions = {},
4550+
options: CreateWriteStreamOptionsInternal = {},
45354551
): void {
45364552
options.metadata ??= {};
45374553

@@ -4545,6 +4561,7 @@ class File extends ServiceObject<File, FileMetadata> {
45454561
uploadType: 'multipart',
45464562
},
45474563
url,
4564+
invocationId: options.invocationId,
45484565
[GCCL_GCS_CMD_KEY]: options[GCCL_GCS_CMD_KEY],
45494566
method: 'POST',
45504567
responseType: 'json',

handwritten/storage/src/storage-transport.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export interface StorageQueryParameters extends StandardStorageQueryParams {
4949

5050
export interface StorageRequestOptions extends GaxiosOptions {
5151
[GCCL_GCS_CMD_KEY]?: string;
52+
invocationId?: string;
5253
interceptors?: GaxiosInterceptor<GaxiosOptionsPrepared>[];
5354
autoPaginate?: boolean;
5455
autoPaginateVal?: boolean;
@@ -254,7 +255,10 @@ export class StorageTransport {
254255
}
255256

256257
#prepareHeaders(reqOpts: StorageRequestOptions): Record<string, string> {
257-
const headersObj = this.#buildRequestHeaders(reqOpts.headers);
258+
const headersObj = this.#buildRequestHeaders(
259+
reqOpts.headers,
260+
reqOpts.invocationId,
261+
);
258262

259263
if (reqOpts[GCCL_GCS_CMD_KEY]) {
260264
const current = headersObj.get('x-goog-api-client') || '';
@@ -299,12 +303,16 @@ export class StorageTransport {
299303
return searchParams.toString();
300304
};
301305

302-
#buildRequestHeaders(requestHeaders = {}) {
303-
const headers = new Headers(requestHeaders);
306+
#buildRequestHeaders(
307+
reqHeaders?: GaxiosOptions['headers'],
308+
invocationId?: string,
309+
) {
310+
const headers = new Headers(reqHeaders);
304311
headers.set('User-Agent', this.#getUserAgentString());
312+
const finalInvocationId = invocationId || randomUUID();
305313
headers.set(
306314
'x-goog-api-client',
307-
`${getRuntimeTrackingString()} gccl/${this.packageJson.version}-${getModuleFormat()} gccl-invocation-id/${randomUUID()}`,
315+
`${getRuntimeTrackingString()} gccl/${this.packageJson.version}-${getModuleFormat()} gccl-invocation-id/${finalInvocationId}`,
308316
);
309317
return headers;
310318
}

handwritten/storage/system-test/storage.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3260,6 +3260,42 @@ describe('storage', function () {
32603260

32613261
assert.strictEqual(called, true);
32623262
});
3263+
3264+
it('should maintain the same invocationId across the upload lifecycle', async () => {
3265+
const invocationIds: string[] = [];
3266+
3267+
const originalRequest = bucket.storageTransport.authClient.request.bind(
3268+
bucket.storageTransport.authClient,
3269+
);
3270+
3271+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3272+
bucket.storageTransport.authClient.request = async (config: any) => {
3273+
const headers = config.headers || {};
3274+
const apiHeaderKey = Object.keys(headers).find(
3275+
key => key.toLowerCase() === 'x-goog-api-client',
3276+
);
3277+
3278+
if (apiHeaderKey) {
3279+
const val = headers[apiHeaderKey];
3280+
const match = val.match(/gccl-invocation-id\/([a-f0-9-]+)/);
3281+
if (match) {
3282+
invocationIds.push(match[1]);
3283+
}
3284+
}
3285+
return originalRequest(config);
3286+
};
3287+
3288+
try {
3289+
const destination = `test-id-${Date.now()}.txt`;
3290+
await bucket.upload(FILES.big.path, {destination, resumable: false});
3291+
3292+
assert.ok(invocationIds.length >= 1);
3293+
const uniqueIds = [...new Set(invocationIds)];
3294+
assert.strictEqual(uniqueIds.length, 1);
3295+
} finally {
3296+
bucket.storageTransport.authClient.request = originalRequest;
3297+
}
3298+
});
32633299
});
32643300

32653301
describe('channels', () => {

0 commit comments

Comments
 (0)