From 6f2870385dbe118ae43be11656a562cf3a4a3f0f Mon Sep 17 00:00:00 2001 From: Denis DelGrosso Date: Tue, 19 Apr 2022 20:45:45 +0000 Subject: [PATCH 1/5] feat: add x-goog-api-client headers for retry metrics --- src/gcs-resumable-upload.ts | 8 +++++++- src/nodejs-common/service.ts | 5 ++++- test/headers.ts | 2 +- test/nodejs-common/service.ts | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/gcs-resumable-upload.ts b/src/gcs-resumable-upload.ts index add6d163d..c152a77f8 100644 --- a/src/gcs-resumable-upload.ts +++ b/src/gcs-resumable-upload.ts @@ -27,6 +27,8 @@ import {GoogleAuth, GoogleAuthOptions} from 'google-auth-library'; import {Readable, Writable} from 'stream'; import retry = require('async-retry'); import {RetryOptions, PreconditionOptions} from './storage'; +import * as packageJson from '../package.json'; +import * as uuid from 'uuid'; const NOT_FOUND_STATUS_CODE = 404; const TERMINATED_UPLOAD_STATUS_CODE = 410; @@ -541,7 +543,11 @@ export class Upload extends Writable { this.params ), data: metadata, - headers: {}, + headers: { + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${ + packageJson.version + } gccl-invocation-id/${uuid.v4()}`, + }, }; if (metadata.contentLength) { diff --git a/src/nodejs-common/service.ts b/src/nodejs-common/service.ts index 2874fb6a2..4235ce832 100644 --- a/src/nodejs-common/service.ts +++ b/src/nodejs-common/service.ts @@ -17,6 +17,7 @@ import arrify = require('arrify'); import * as extend from 'extend'; import {AuthClient, GoogleAuth, GoogleAuthOptions} from 'google-auth-library'; import * as r from 'teeny-request'; +import * as uuid from 'uuid'; import {Interceptor} from './service-object'; import { @@ -242,7 +243,9 @@ export class Service { } reqOpts.headers = extend({}, reqOpts.headers, { 'User-Agent': userAgent, - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${pkg.version}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${ + pkg.version + } gccl-invocation-id/${uuid.v4()}`, }); if (reqOpts.shouldReturnStream) { diff --git a/test/headers.ts b/test/headers.ts index 1d8f1dd74..346d2e535 100644 --- a/test/headers.ts +++ b/test/headers.ts @@ -60,7 +60,7 @@ describe('headers', () => { if (err !== error) throw err; } assert.ok( - /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+$/.test( + /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+ gccl-invocation-id\/[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}$/.test( requests[0].headers['x-goog-api-client'] ) ); diff --git a/test/nodejs-common/service.ts b/test/nodejs-common/service.ts index 0674592c1..1cf8f5687 100644 --- a/test/nodejs-common/service.ts +++ b/test/nodejs-common/service.ts @@ -31,6 +31,7 @@ import { util, Util, } from '../../src/nodejs-common/util'; +import * as uuid from 'uuid'; proxyquire.noPreserveCache(); @@ -514,7 +515,9 @@ describe('Service', () => { const pkg = service.packageJson; assert.strictEqual( reqOpts.headers!['x-goog-api-client'], - `gl-node/${process.versions.node} gccl/${pkg.version}` + `gl-node/${process.versions.node} gccl/${ + pkg.version + } gccl-invocation-id/${uuid.v4()}` ); done(); }; From 1ea0eed12dc6d970395b9a344ff1420b52bbebbc Mon Sep 17 00:00:00 2001 From: Denis DelGrosso Date: Fri, 6 May 2022 15:57:29 +0000 Subject: [PATCH 2/5] update tests, add invocation id to more places --- src/gcs-resumable-upload.ts | 41 ++++++++---- src/nodejs-common/service.ts | 5 +- src/nodejs-common/util.ts | 16 ++++- test/gcs-resumable-upload.ts | 119 ++++++++++++++++++++++++---------- test/headers.ts | 2 +- test/nodejs-common/service.ts | 5 +- test/nodejs-common/util.ts | 10 +-- 7 files changed, 138 insertions(+), 60 deletions(-) diff --git a/src/gcs-resumable-upload.ts b/src/gcs-resumable-upload.ts index c152a77f8..fccdbbf1f 100644 --- a/src/gcs-resumable-upload.ts +++ b/src/gcs-resumable-upload.ts @@ -264,6 +264,7 @@ export class Upload extends Writable { contentLength: number | '*'; retryOptions: RetryOptions; timeOfFirstRequest: number; + currentInvocationId = uuid.v4(); private upstreamChunkBuffer: Buffer = Buffer.alloc(0); private chunkBufferEncoding?: BufferEncoding = undefined; private numChunksReadInRequest = 0; @@ -531,7 +532,12 @@ export class Upload extends Writable { protected async createURIAsync(): Promise { const metadata = this.metadata; + // Create a new invocation id if this is not a retry call + if (this.numRetries === 0) { + this.currentInvocationId = uuid.v4(); + } + // Check if headers already exist before creating new ones const reqOpts: GaxiosOptions = { method: 'POST', url: [this.baseURI, this.bucket, 'o'].join('/'), @@ -544,9 +550,7 @@ export class Upload extends Writable { ), data: metadata, headers: { - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${ - packageJson.version - } gccl-invocation-id/${uuid.v4()}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId}`, }, }; @@ -713,20 +717,25 @@ export class Upload extends Writable { }, }); - let headers: GaxiosOptions['headers'] = {}; + // Create a new invocation id if this is not a retry call + if (this.numRetries === 0) { + this.currentInvocationId = uuid.v4(); + } + + const headers: GaxiosOptions['headers'] = { + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId}`, + }; // If using multiple chunk upload, set appropriate header if (multiChunkMode && expectedUploadSize) { // The '-1' is because the ending byte is inclusive in the request. const endingByte = expectedUploadSize + this.numBytesWritten - 1; - headers = { - 'Content-Length': expectedUploadSize, - 'Content-Range': `bytes ${this.offset}-${endingByte}/${this.contentLength}`, - }; + headers['Content-Length'] = expectedUploadSize; + headers[ + 'Content-Range' + ] = `bytes ${this.offset}-${endingByte}/${this.contentLength}`; } else { - headers = { - 'Content-Range': `bytes ${this.offset}-*/${this.contentLength}`, - }; + headers['Content-Range'] = `bytes ${this.offset}-*/${this.contentLength}`; } const reqOpts: GaxiosOptions = { @@ -852,10 +861,18 @@ export class Upload extends Writable { } private async getAndSetOffset() { + // Create a new invocation id if this is not a retry call + if (this.numRetries === 0) { + this.currentInvocationId = uuid.v4(); + } const opts: GaxiosOptions = { method: 'PUT', url: this.uri!, - headers: {'Content-Length': 0, 'Content-Range': 'bytes */*'}, + headers: { + 'Content-Length': 0, + 'Content-Range': 'bytes */*', + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId}`, + }, }; try { const resp = await this.makeRequest(opts); diff --git a/src/nodejs-common/service.ts b/src/nodejs-common/service.ts index 4235ce832..2874fb6a2 100644 --- a/src/nodejs-common/service.ts +++ b/src/nodejs-common/service.ts @@ -17,7 +17,6 @@ import arrify = require('arrify'); import * as extend from 'extend'; import {AuthClient, GoogleAuth, GoogleAuthOptions} from 'google-auth-library'; import * as r from 'teeny-request'; -import * as uuid from 'uuid'; import {Interceptor} from './service-object'; import { @@ -243,9 +242,7 @@ export class Service { } reqOpts.headers = extend({}, reqOpts.headers, { 'User-Agent': userAgent, - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${ - pkg.version - } gccl-invocation-id/${uuid.v4()}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${pkg.version}`, }); if (reqOpts.shouldReturnStream) { diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index a0d784002..ea5672805 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -27,13 +27,14 @@ import * as r from 'teeny-request'; import * as retryRequest from 'retry-request'; import {Duplex, DuplexOptions, Readable, Transform, Writable} from 'stream'; import {teenyRequest} from 'teeny-request'; - import {Interceptor} from './service-object'; +import * as uuid from 'uuid'; +import * as packageJson from '../../package.json'; // eslint-disable-next-line @typescript-eslint/no-var-requires const duplexify: DuplexifyConstructor = require('duplexify'); -const requestDefaults = { +const requestDefaults: r.CoreOptions = { timeout: 60000, gzip: true, forever: true, @@ -522,6 +523,7 @@ export class Util { return; } + requestDefaults.headers = util._getDefaultHeaders(); const request = teenyRequest.defaults(requestDefaults); request(authenticatedReqOpts!, (err, resp, body) => { util.handleResp(err, resp, body, (err, data) => { @@ -797,6 +799,7 @@ export class Util { maxRetryValue = config.retryOptions.maxRetries; } + requestDefaults.headers = this._getDefaultHeaders(); const options = { request: teenyRequest.defaults(requestDefaults), retries: autoRetryValue !== false ? maxRetryValue : 0, @@ -945,6 +948,15 @@ export class Util { ? [{} as T, optionsOrCallback as C] : [optionsOrCallback as T, cb as C]; } + + _getDefaultHeaders() { + return { + 'User-Agent': util.getUserAgentFromPackageJson(packageJson), + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${ + packageJson.version + } gccl-invocation-id/${uuid.v4()}`, + }; + } } /** diff --git a/test/gcs-resumable-upload.ts b/test/gcs-resumable-upload.ts index eaa273a7c..955a51ea8 100644 --- a/test/gcs-resumable-upload.ts +++ b/test/gcs-resumable-upload.ts @@ -67,6 +67,8 @@ const RESUMABLE_INCOMPLETE_STATUS_CODE = 308; /** 256 KiB */ const CHUNK_SIZE_MULTIPLE = 2 ** 18; const queryPath = '/?userProject=user-project-id'; +const X_GOOG_API_HEADER_REGEX = + /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+ gccl-invocation-id\/[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}$/; function mockAuthorizeRequest( code = 200, @@ -1106,9 +1108,14 @@ describe('gcs-resumable-upload', () => { await up.startUploading(); - assert.deepEqual(reqOpts.headers, { - 'Content-Range': `bytes ${OFFSET}-*/${CONTENT_LENGTH}`, - }); + assert(reqOpts.headers); + assert.equal( + reqOpts.headers['Content-Range'], + `bytes ${OFFSET}-*/${CONTENT_LENGTH}` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) + ); const data = await getAllDataFromRequest(); @@ -1120,9 +1127,11 @@ describe('gcs-resumable-upload', () => { await up.startUploading(); - assert.deepEqual(reqOpts.headers, { - 'Content-Range': 'bytes 0-*/*', - }); + assert(reqOpts.headers); + assert.equal(reqOpts.headers['Content-Range'], 'bytes 0-*/*'); + assert.ok( + X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) + ); const data = await getAllDataFromRequest(); @@ -1147,10 +1156,15 @@ describe('gcs-resumable-upload', () => { await up.startUploading(); const endByte = OFFSET + CHUNK_SIZE - 1; - assert.deepEqual(reqOpts.headers, { - 'Content-Length': CHUNK_SIZE, - 'Content-Range': `bytes ${OFFSET}-${endByte}/${CONTENT_LENGTH}`, - }); + assert(reqOpts.headers); + assert.equal(reqOpts.headers['Content-Length'], CHUNK_SIZE); + assert.equal( + reqOpts.headers['Content-Range'], + `bytes ${OFFSET}-${endByte}/${CONTENT_LENGTH}` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) + ); const data = await getAllDataFromRequest(); @@ -1166,10 +1180,15 @@ describe('gcs-resumable-upload', () => { await up.startUploading(); const endByte = OFFSET + CHUNK_SIZE - 1; - assert.deepEqual(reqOpts.headers, { - 'Content-Length': CHUNK_SIZE, - 'Content-Range': `bytes ${OFFSET}-${endByte}/*`, - }); + assert(reqOpts.headers); + assert.equal(reqOpts.headers['Content-Length'], CHUNK_SIZE); + assert.equal( + reqOpts.headers['Content-Range'], + `bytes ${OFFSET}-${endByte}/*` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) + ); const data = await getAllDataFromRequest(); @@ -1188,10 +1207,18 @@ describe('gcs-resumable-upload', () => { await up.startUploading(); const endByte = CONTENT_LENGTH - NUM_BYTES_WRITTEN + OFFSET - 1; - assert.deepEqual(reqOpts.headers, { - 'Content-Length': CONTENT_LENGTH - NUM_BYTES_WRITTEN, - 'Content-Range': `bytes ${OFFSET}-${endByte}/${CONTENT_LENGTH}`, - }); + assert(reqOpts.headers); + assert.equal( + reqOpts.headers['Content-Length'], + CONTENT_LENGTH - NUM_BYTES_WRITTEN + ); + assert.equal( + reqOpts.headers['Content-Range'], + `bytes ${OFFSET}-${endByte}/${CONTENT_LENGTH}` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) + ); const data = await getAllDataFromRequest(); assert.equal(data.byteLength, CONTENT_LENGTH - NUM_BYTES_WRITTEN); @@ -1418,10 +1445,12 @@ describe('gcs-resumable-upload', () => { up.makeRequest = async (reqOpts: GaxiosOptions) => { assert.strictEqual(reqOpts.method, 'PUT'); assert.strictEqual(reqOpts.url, URI); - assert.deepStrictEqual(reqOpts.headers, { - 'Content-Length': 0, - 'Content-Range': 'bytes */*', - }); + assert(reqOpts.headers); + assert.equal(reqOpts.headers['Content-Length'], 0); + assert.equal(reqOpts.headers['Content-Range'], 'bytes */*'); + assert.ok( + X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) + ); done(); return {}; }; @@ -2345,9 +2374,16 @@ describe('gcs-resumable-upload', () => { assert(request.chunkWritesInRequest > 1); assert.equal(request.dataReceived, CONTENT_LENGTH); - assert.deepStrictEqual(request.opts.headers, { - 'Content-Range': `bytes 0-*/${CONTENT_LENGTH}`, - }); + assert(request.opts.headers); + assert.equal( + request.opts.headers['Content-Range'], + `bytes 0-*/${CONTENT_LENGTH}` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test( + request.opts.headers['x-goog-api-client'] + ) + ); done(); }); @@ -2504,19 +2540,36 @@ describe('gcs-resumable-upload', () => { const endByte = offset + LAST_REQUEST_SIZE - 1; assert.equal(request.dataReceived, LAST_REQUEST_SIZE); - assert.deepStrictEqual(request.opts.headers, { - 'Content-Length': LAST_REQUEST_SIZE, - 'Content-Range': `bytes ${offset}-${endByte}/${CONTENT_LENGTH}`, - }); + assert(request.opts.headers); + assert.equal( + request.opts.headers['Content-Length'], + LAST_REQUEST_SIZE + ); + assert.equal( + request.opts.headers['Content-Range'], + `bytes ${offset}-${endByte}/${CONTENT_LENGTH}` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test( + request.opts.headers['x-goog-api-client'] + ) + ); } else { // The preceding chunks const endByte = offset + CHUNK_SIZE - 1; assert.equal(request.dataReceived, CHUNK_SIZE); - assert.deepStrictEqual(request.opts.headers, { - 'Content-Length': CHUNK_SIZE, - 'Content-Range': `bytes ${offset}-${endByte}/${CONTENT_LENGTH}`, - }); + assert(request.opts.headers); + assert.equal(request.opts.headers['Content-Length'], CHUNK_SIZE); + assert.equal( + request.opts.headers['Content-Range'], + `bytes ${offset}-${endByte}/${CONTENT_LENGTH}` + ); + assert.ok( + X_GOOG_API_HEADER_REGEX.test( + request.opts.headers['x-goog-api-client'] + ) + ); } } diff --git a/test/headers.ts b/test/headers.ts index 346d2e535..1d8f1dd74 100644 --- a/test/headers.ts +++ b/test/headers.ts @@ -60,7 +60,7 @@ describe('headers', () => { if (err !== error) throw err; } assert.ok( - /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+ gccl-invocation-id\/[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}$/.test( + /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+$/.test( requests[0].headers['x-goog-api-client'] ) ); diff --git a/test/nodejs-common/service.ts b/test/nodejs-common/service.ts index 1cf8f5687..0674592c1 100644 --- a/test/nodejs-common/service.ts +++ b/test/nodejs-common/service.ts @@ -31,7 +31,6 @@ import { util, Util, } from '../../src/nodejs-common/util'; -import * as uuid from 'uuid'; proxyquire.noPreserveCache(); @@ -515,9 +514,7 @@ describe('Service', () => { const pkg = service.packageJson; assert.strictEqual( reqOpts.headers!['x-goog-api-client'], - `gl-node/${process.versions.node} gccl/${ - pkg.version - } gccl-invocation-id/${uuid.v4()}` + `gl-node/${process.versions.node} gccl/${pkg.version}` ); done(); }; diff --git a/test/nodejs-common/util.ts b/test/nodejs-common/util.ts index 3c5fb4c62..6450bef93 100644 --- a/test/nodejs-common/util.ts +++ b/test/nodejs-common/util.ts @@ -75,9 +75,12 @@ function fakeRequest() { return (requestOverride || teenyRequest).apply(null, arguments); } -fakeRequest.defaults = () => { - // Ignore the default values, so we don't have to test for them in every API - // call. +fakeRequest.defaults = (defaults: r.CoreOptions) => { + assert.ok( + /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+ gccl-invocation-id\/[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}$/.test( + defaults.headers!['x-goog-api-client'] + ) + ); return fakeRequest; }; @@ -503,7 +506,6 @@ describe('common/util', () => { assert.strictEqual(request.qs.uploadType, 'multipart'); assert.strictEqual(request.timeout, 0); assert.strictEqual(request.maxRetries, 0); - assert.strictEqual(Array.isArray(request.multipart), true); const mp = request.multipart as r.RequestPart[]; From d798c0204db8ebecb2b54570de7c0b87882e3165 Mon Sep 17 00:00:00 2001 From: Denis DelGrosso Date: Mon, 9 May 2022 17:30:27 +0000 Subject: [PATCH 3/5] add invocation id and tests to gcs resumable upload --- src/gcs-resumable-upload.ts | 32 ++++++++++---------- test/gcs-resumable-upload.ts | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/src/gcs-resumable-upload.ts b/src/gcs-resumable-upload.ts index fccdbbf1f..7dbdc08fb 100644 --- a/src/gcs-resumable-upload.ts +++ b/src/gcs-resumable-upload.ts @@ -264,7 +264,11 @@ export class Upload extends Writable { contentLength: number | '*'; retryOptions: RetryOptions; timeOfFirstRequest: number; - currentInvocationId = uuid.v4(); + private currentInvocationId = { + chunk: uuid.v4(), + uri: uuid.v4(), + offset: uuid.v4(), + }; private upstreamChunkBuffer: Buffer = Buffer.alloc(0); private chunkBufferEncoding?: BufferEncoding = undefined; private numChunksReadInRequest = 0; @@ -532,10 +536,6 @@ export class Upload extends Writable { protected async createURIAsync(): Promise { const metadata = this.metadata; - // Create a new invocation id if this is not a retry call - if (this.numRetries === 0) { - this.currentInvocationId = uuid.v4(); - } // Check if headers already exist before creating new ones const reqOpts: GaxiosOptions = { @@ -550,7 +550,7 @@ export class Upload extends Writable { ), data: metadata, headers: { - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId.uri}`, }, }; @@ -582,6 +582,8 @@ export class Upload extends Writable { async (bail: (err: Error) => void) => { try { const res = await this.makeRequest(reqOpts); + // We have successfully got a URI we can now create a new invocation id + this.currentInvocationId.uri = uuid.v4(); return res.headers.location; } catch (err) { const e = err as GaxiosError; @@ -717,13 +719,8 @@ export class Upload extends Writable { }, }); - // Create a new invocation id if this is not a retry call - if (this.numRetries === 0) { - this.currentInvocationId = uuid.v4(); - } - const headers: GaxiosOptions['headers'] = { - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId.chunk}`, }; // If using multiple chunk upload, set appropriate header @@ -765,6 +762,9 @@ export class Upload extends Writable { return; } + // At this point we can safely create a new id for the chunk + this.currentInvocationId.chunk = uuid.v4(); + const shouldContinueWithNextMultiChunkRequest = this.chunkSize && resp.status === RESUMABLE_INCOMPLETE_STATUS_CODE && @@ -861,21 +861,19 @@ export class Upload extends Writable { } private async getAndSetOffset() { - // Create a new invocation id if this is not a retry call - if (this.numRetries === 0) { - this.currentInvocationId = uuid.v4(); - } const opts: GaxiosOptions = { method: 'PUT', url: this.uri!, headers: { 'Content-Length': 0, 'Content-Range': 'bytes */*', - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${packageJson.version} gccl-invocation-id/${this.currentInvocationId.offset}`, }, }; try { const resp = await this.makeRequest(opts); + // Successfully got the offset we can now create a new offset invocation id + this.currentInvocationId.offset = uuid.v4(); if (resp.status === RESUMABLE_INCOMPLETE_STATUS_CODE) { if (resp.headers.range) { const range = resp.headers.range as string; diff --git a/test/gcs-resumable-upload.ts b/test/gcs-resumable-upload.ts index 955a51ea8..57950e338 100644 --- a/test/gcs-resumable-upload.ts +++ b/test/gcs-resumable-upload.ts @@ -906,6 +906,15 @@ describe('gcs-resumable-upload', () => { done(); }); }); + + it('currentInvocationId.uri should remain the same on error', done => { + const beforeCallInvocationId = up.currentInvocationId.uri; + up.createURI((err: Error) => { + assert(err); + assert.equal(beforeCallInvocationId, up.currentInvocationId.uri); + done(); + }); + }); }); describe('success', () => { @@ -942,6 +951,14 @@ describe('gcs-resumable-upload', () => { done(); }); }); + + it('currentInvocationId.uri should be different after success', done => { + const beforeCallInvocationId = up.currentInvocationId.uri; + up.createURI(() => { + assert.notEqual(beforeCallInvocationId, up.currentInvocationId.uri); + done(); + }); + }); }); }); @@ -1348,6 +1365,26 @@ describe('gcs-resumable-upload', () => { up.responseHandler(RESP); }); + + it('currentInvocationId.chunk should be different after success', done => { + const beforeCallInvocationId = up.currentInvocationId.chunk; + const RESP = {data: '', status: 200}; + up.once('prepareFinish', () => { + assert.notEqual(beforeCallInvocationId, up.currentInvocationId.chunk); + done(); + }); + up.responseHandler(RESP); + }); + + it('currentInvocationId.chunk should be the same after error', done => { + const beforeCallInvocationId = up.currentInvocationId.chunk; + const RESP = {data: {error: new Error('Error.')}}; + up.destroy = () => { + assert.equal(beforeCallInvocationId, up.currentInvocationId.chunk); + done(); + }; + up.responseHandler(RESP); + }); }); describe('#ensureUploadingSameObject', () => { @@ -1457,6 +1494,27 @@ describe('gcs-resumable-upload', () => { up.getAndSetOffset(); }); + it('currentInvocationId.offset should be different after success', async () => { + const beforeCallInvocationId = up.currentInvocationId.offset; + up.makeRequest = () => { + return {}; + }; + await up.getAndSetOffset(); + assert.notEqual(beforeCallInvocationId, up.currentInvocationId.offset); + }); + + it('currentInvocationId.offset should be the same on error', async done => { + const beforeCallInvocationId = up.currentInvocationId.offset; + up.destroy = () => { + assert.equal(beforeCallInvocationId, up.currentInvocationId.offset); + done(); + }; + up.makeRequest = () => { + throw new Error() as GaxiosError; + }; + await up.getAndSetOffset(); + }); + describe('restart on 404', () => { const RESP = {status: 404} as GaxiosResponse; const ERROR = new Error(':(') as GaxiosError; From 61a4593aeefeda9248856e8261bc9e22e45575c4 Mon Sep 17 00:00:00 2001 From: Denis DelGrosso Date: Mon, 9 May 2022 17:43:08 +0000 Subject: [PATCH 4/5] fix event naming --- test/gcs-resumable-upload.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/gcs-resumable-upload.ts b/test/gcs-resumable-upload.ts index 57950e338..65b052bc0 100644 --- a/test/gcs-resumable-upload.ts +++ b/test/gcs-resumable-upload.ts @@ -1369,7 +1369,7 @@ describe('gcs-resumable-upload', () => { it('currentInvocationId.chunk should be different after success', done => { const beforeCallInvocationId = up.currentInvocationId.chunk; const RESP = {data: '', status: 200}; - up.once('prepareFinish', () => { + up.on('uploadFinished', () => { assert.notEqual(beforeCallInvocationId, up.currentInvocationId.chunk); done(); }); From 49784669b5358075230aced3a8d4d403c10f160c Mon Sep 17 00:00:00 2001 From: Denis DelGrosso Date: Tue, 10 May 2022 14:11:51 +0000 Subject: [PATCH 5/5] fix method of pulling package.json, update regex --- src/gcs-resumable-upload.ts | 9 ++++++++- src/nodejs-common/service.ts | 5 ++++- test/gcs-resumable-upload.ts | 2 +- test/headers.ts | 2 +- test/nodejs-common/service.ts | 6 +++--- test/nodejs-common/util.ts | 2 +- 6 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/gcs-resumable-upload.ts b/src/gcs-resumable-upload.ts index 7dbdc08fb..6b8360932 100644 --- a/src/gcs-resumable-upload.ts +++ b/src/gcs-resumable-upload.ts @@ -27,13 +27,20 @@ import {GoogleAuth, GoogleAuthOptions} from 'google-auth-library'; import {Readable, Writable} from 'stream'; import retry = require('async-retry'); import {RetryOptions, PreconditionOptions} from './storage'; -import * as packageJson from '../package.json'; import * as uuid from 'uuid'; const NOT_FOUND_STATUS_CODE = 404; const TERMINATED_UPLOAD_STATUS_CODE = 410; const RESUMABLE_INCOMPLETE_STATUS_CODE = 308; const DEFAULT_API_ENDPOINT_REGEX = /.*\.googleapis\.com/; +let packageJson: ReturnType = {}; +try { + // if requiring from 'build' (default) + packageJson = require('../../package.json'); +} catch (e) { + // if requiring directly from TypeScript context + packageJson = require('../package.json'); +} export const PROTOCOL_REGEX = /^(\w*):\/\//; diff --git a/src/nodejs-common/service.ts b/src/nodejs-common/service.ts index 2874fb6a2..4235ce832 100644 --- a/src/nodejs-common/service.ts +++ b/src/nodejs-common/service.ts @@ -17,6 +17,7 @@ import arrify = require('arrify'); import * as extend from 'extend'; import {AuthClient, GoogleAuth, GoogleAuthOptions} from 'google-auth-library'; import * as r from 'teeny-request'; +import * as uuid from 'uuid'; import {Interceptor} from './service-object'; import { @@ -242,7 +243,9 @@ export class Service { } reqOpts.headers = extend({}, reqOpts.headers, { 'User-Agent': userAgent, - 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${pkg.version}`, + 'x-goog-api-client': `gl-node/${process.versions.node} gccl/${ + pkg.version + } gccl-invocation-id/${uuid.v4()}`, }); if (reqOpts.shouldReturnStream) { diff --git a/test/gcs-resumable-upload.ts b/test/gcs-resumable-upload.ts index 65b052bc0..f28222d77 100644 --- a/test/gcs-resumable-upload.ts +++ b/test/gcs-resumable-upload.ts @@ -68,7 +68,7 @@ const RESUMABLE_INCOMPLETE_STATUS_CODE = 308; const CHUNK_SIZE_MULTIPLE = 2 ** 18; const queryPath = '/?userProject=user-project-id'; const X_GOOG_API_HEADER_REGEX = - /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+ gccl-invocation-id\/[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}$/; + /^gl-node\/(?[^W]+) gccl\/(?[^W]+) gccl-invocation-id\/(?[^W]+)$/; function mockAuthorizeRequest( code = 200, diff --git a/test/headers.ts b/test/headers.ts index 1d8f1dd74..0d9249122 100644 --- a/test/headers.ts +++ b/test/headers.ts @@ -60,7 +60,7 @@ describe('headers', () => { if (err !== error) throw err; } assert.ok( - /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+$/.test( + /^gl-node\/(?[^W]+) gccl\/(?[^W]+) gccl-invocation-id\/(?[^W]+)$/.test( requests[0].headers['x-goog-api-client'] ) ); diff --git a/test/nodejs-common/service.ts b/test/nodejs-common/service.ts index 0674592c1..363164238 100644 --- a/test/nodejs-common/service.ts +++ b/test/nodejs-common/service.ts @@ -512,10 +512,10 @@ describe('Service', () => { it('should add the api-client header', done => { service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { const pkg = service.packageJson; - assert.strictEqual( - reqOpts.headers!['x-goog-api-client'], - `gl-node/${process.versions.node} gccl/${pkg.version}` + const r = new RegExp( + `^gl-node/${process.versions.node} gccl/${pkg.version} gccl-invocation-id/(?[^W]+)$` ); + assert.ok(r.test(reqOpts.headers!['x-goog-api-client'])); done(); }; diff --git a/test/nodejs-common/util.ts b/test/nodejs-common/util.ts index 6450bef93..113cb853c 100644 --- a/test/nodejs-common/util.ts +++ b/test/nodejs-common/util.ts @@ -77,7 +77,7 @@ function fakeRequest() { fakeRequest.defaults = (defaults: r.CoreOptions) => { assert.ok( - /^gl-node\/[0-9]+\.[0-9]+\.[-.\w]+ gccl\/[0-9]+\.[0-9]+\.[-.\w]+ gccl-invocation-id\/[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}$/.test( + /^gl-node\/(?[^W]+) gccl\/(?[^W]+) gccl-invocation-id\/(?[^W]+)$/.test( defaults.headers!['x-goog-api-client'] ) );