From 3701d12037cdd0b2c6cfe1e42890ca0ab4db9c80 Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Mon, 1 Jun 2020 13:03:35 -0700 Subject: [PATCH 1/8] more informtion for exceed timeout error --- src/normalCalls/retries.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index 31c50e7cd..f3cdfb26f 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -69,8 +69,9 @@ export function retryable( let timeoutId: ReturnType | null; let now = new Date(); let deadline: number; + const startTime = now.getTime(); if (retry.backoffSettings.totalTimeoutMillis) { - deadline = now.getTime() + retry.backoffSettings.totalTimeoutMillis; + deadline = startTime + retry.backoffSettings.totalTimeoutMillis; } let retries = 0; const maxRetries = retry.backoffSettings.maxRetries!; @@ -80,8 +81,10 @@ export function retryable( function repeat() { timeoutId = null; if (deadline && now.getTime() >= deadline) { + const timeCost = now.getTime() - startTime; + const funcName = func.name; const error = new GoogleError( - 'Retry total timeout exceeded before any response was received' + `funcName takes ${timeCost} milliseconds which exceeds retry total timeout ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received. The retry settings is ${retry.backoffSettings}.` ); error.code = Status.DEADLINE_EXCEEDED; callback(error); From e0577336d10a4b2d2bc9c2866459d13579714165 Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Mon, 1 Jun 2020 13:04:07 -0700 Subject: [PATCH 2/8] fix --- src/normalCalls/retries.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index f3cdfb26f..10c8c13eb 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -84,7 +84,7 @@ export function retryable( const timeCost = now.getTime() - startTime; const funcName = func.name; const error = new GoogleError( - `funcName takes ${timeCost} milliseconds which exceeds retry total timeout ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received. The retry settings is ${retry.backoffSettings}.` + `${funcName} takes ${timeCost} milliseconds which exceeds retry total timeout ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received. The retry settings is ${retry.backoffSettings}.` ); error.code = Status.DEADLINE_EXCEEDED; callback(error); From 08f9ca8e3acd77e27205f8e4dffca096d72a0d45 Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Mon, 1 Jun 2020 17:00:25 -0700 Subject: [PATCH 3/8] add unit test --- src/normalCalls/retries.ts | 6 +++++- test/unit/apiCallable.ts | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index 10c8c13eb..7d6e85c81 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -84,7 +84,11 @@ export function retryable( const timeCost = now.getTime() - startTime; const funcName = func.name; const error = new GoogleError( - `${funcName} takes ${timeCost} milliseconds which exceeds retry total timeout ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received. The retry settings is ${retry.backoffSettings}.` + `Function ${funcName} takes ${timeCost} milliseconds which exceeds retry total timeout ${ + retry.backoffSettings.totalTimeoutMillis + } milliseconds before any response was received. The retry settings is ${JSON.stringify( + retry.backoffSettings + )}.` ); error.code = Status.DEADLINE_EXCEEDED; callback(error); diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts index 6f9d0876e..7b049cc4e 100644 --- a/test/unit/apiCallable.ts +++ b/test/unit/apiCallable.ts @@ -364,6 +364,20 @@ describe('retryable', () => { }); }); + it('retry fails for exceeding total timeout', done => { + const spy = sinon.spy(fail); + const apiCall = createApiCall(spy, settings); + apiCall({}, undefined, err => { + assert.ok(err instanceof GoogleError); + assert.strictEqual( + err.message, + 'Function takes 100 milliseconds which exceeds retry total timeout 100 milliseconds before any response was received. The retry settings is {"initialRetryDelayMillis":0,"retryDelayMultiplier":0,"maxRetryDelayMillis":0,"initialRpcTimeoutMillis":0,"rpcTimeoutMultiplier":0,"maxRpcTimeoutMillis":0,"totalTimeoutMillis":100}.' + ); + assert.strictEqual(err!.code, status.DEADLINE_EXCEEDED); + done(); + }); + }); + // maxRetries is unsupported, and intended for internal use only. it('errors when totalTimeoutMillis and maxRetries set', done => { const maxRetries = 5; From b3963ad083cd2e4abd1dfec4d917a5e4dd0ad04d Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Tue, 2 Jun 2020 10:16:46 -0700 Subject: [PATCH 4/8] feedback --- src/normalCalls/retries.ts | 7 +------ test/unit/apiCallable.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index 7d6e85c81..1cbcdb45a 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -82,13 +82,8 @@ export function retryable( timeoutId = null; if (deadline && now.getTime() >= deadline) { const timeCost = now.getTime() - startTime; - const funcName = func.name; const error = new GoogleError( - `Function ${funcName} takes ${timeCost} milliseconds which exceeds retry total timeout ${ - retry.backoffSettings.totalTimeoutMillis - } milliseconds before any response was received. The retry settings is ${JSON.stringify( - retry.backoffSettings - )}.` + `Function takes ${timeCost} milliseconds which exceeds retry total timeout ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received.` ); error.code = Status.DEADLINE_EXCEEDED; callback(error); diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts index 7b049cc4e..4bd6a33e9 100644 --- a/test/unit/apiCallable.ts +++ b/test/unit/apiCallable.ts @@ -371,7 +371,7 @@ describe('retryable', () => { assert.ok(err instanceof GoogleError); assert.strictEqual( err.message, - 'Function takes 100 milliseconds which exceeds retry total timeout 100 milliseconds before any response was received. The retry settings is {"initialRetryDelayMillis":0,"retryDelayMultiplier":0,"maxRetryDelayMillis":0,"initialRpcTimeoutMillis":0,"rpcTimeoutMultiplier":0,"maxRpcTimeoutMillis":0,"totalTimeoutMillis":100}.' + 'Function takes 100 milliseconds which exceeds retry total timeout 100 milliseconds before any response was received.' ); assert.strictEqual(err!.code, status.DEADLINE_EXCEEDED); done(); From 0794afc24eafb0d7474470adc09fee58ba8a48a2 Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Tue, 2 Jun 2020 10:31:46 -0700 Subject: [PATCH 5/8] reformat error message --- src/normalCalls/retries.ts | 6 ++---- test/unit/apiCallable.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index 1cbcdb45a..4ef9457e1 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -69,9 +69,8 @@ export function retryable( let timeoutId: ReturnType | null; let now = new Date(); let deadline: number; - const startTime = now.getTime(); if (retry.backoffSettings.totalTimeoutMillis) { - deadline = startTime + retry.backoffSettings.totalTimeoutMillis; + deadline = now.getTime() + retry.backoffSettings.totalTimeoutMillis; } let retries = 0; const maxRetries = retry.backoffSettings.maxRetries!; @@ -81,9 +80,8 @@ export function retryable( function repeat() { timeoutId = null; if (deadline && now.getTime() >= deadline) { - const timeCost = now.getTime() - startTime; const error = new GoogleError( - `Function takes ${timeCost} milliseconds which exceeds retry total timeout ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received.` + `Retry total timeout exceeded ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received.` ); error.code = Status.DEADLINE_EXCEEDED; callback(error); diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts index 4bd6a33e9..31153839b 100644 --- a/test/unit/apiCallable.ts +++ b/test/unit/apiCallable.ts @@ -371,7 +371,7 @@ describe('retryable', () => { assert.ok(err instanceof GoogleError); assert.strictEqual( err.message, - 'Function takes 100 milliseconds which exceeds retry total timeout 100 milliseconds before any response was received.' + 'Retry total timeout exceeded 100 milliseconds before any response was received.' ); assert.strictEqual(err!.code, status.DEADLINE_EXCEEDED); done(); From 6cfaea8847e206efadd27faaeb2f58d1a15cd13e Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Tue, 2 Jun 2020 14:35:58 -0700 Subject: [PATCH 6/8] pass api name to create api call --- src/createApiCall.ts | 3 ++- src/gax.ts | 8 ++++++++ src/normalCalls/retries.ts | 5 +++-- test/unit/apiCallable.ts | 4 ++-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/createApiCall.ts b/src/createApiCall.ts index 9b785960a..7ed95e537 100644 --- a/src/createApiCall.ts +++ b/src/createApiCall.ts @@ -88,7 +88,8 @@ export function createApiCall( return retryable( func, thisSettings.retry!, - thisSettings.otherArgs as GRPCCallOtherArgs + thisSettings.otherArgs as GRPCCallOtherArgs, + thisSettings.apiName ); } return addTimeoutArg( diff --git a/src/gax.ts b/src/gax.ts index 2c566bc51..7fb05c68e 100644 --- a/src/gax.ts +++ b/src/gax.ts @@ -124,6 +124,7 @@ export interface CallOptions { bundleOptions?: BundleOptions | null; isBundling?: boolean; longrunning?: BackoffSettings; + apiName?: string; } export class CallSettings { @@ -138,6 +139,7 @@ export class CallSettings { bundleOptions?: BundleOptions | null; isBundling: boolean; longrunning?: BackoffSettings; + apiName?: string; /** * @param {Object} settings - An object containing parameters of this settings. @@ -170,6 +172,7 @@ export class CallSettings { this.isBundling = 'isBundling' in settings ? settings.isBundling! : true; this.longrunning = 'longrunning' in settings ? settings.longrunning : undefined; + this.apiName = settings.apiName ?? undefined; } /** @@ -193,6 +196,7 @@ export class CallSettings { let otherArgs = this.otherArgs; let isBundling = this.isBundling; let longrunning = this.longrunning; + let apiName = this.apiName; if ('timeout' in options) { timeout = options.timeout!; } @@ -239,6 +243,9 @@ export class CallSettings { if ('longrunning' in options) { longrunning = options.longrunning; } + if ('apiName' in options) { + apiName = options.apiName; + } return new CallSettings({ timeout, @@ -251,6 +258,7 @@ export class CallSettings { maxResults, otherArgs, isBundling, + apiName, }); } } diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index 4ef9457e1..293e6f24c 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -45,7 +45,8 @@ import {addTimeoutArg} from './timeout'; export function retryable( func: GRPCCall, retry: RetryOptions, - otherArgs: GRPCCallOtherArgs + otherArgs: GRPCCallOtherArgs, + apiName?: string, ): SimpleCallbackFunction { const delayMult = retry.backoffSettings.retryDelayMultiplier; const maxDelay = retry.backoffSettings.maxRetryDelayMillis; @@ -81,7 +82,7 @@ export function retryable( timeoutId = null; if (deadline && now.getTime() >= deadline) { const error = new GoogleError( - `Retry total timeout exceeded ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received.` + `Total timeout of API ${apiName} exceeded ${retry.backoffSettings.totalTimeoutMillis} milliseconds before any response was received.` ); error.code = Status.DEADLINE_EXCEEDED; callback(error); diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts index 31153839b..ecd58a45d 100644 --- a/test/unit/apiCallable.ts +++ b/test/unit/apiCallable.ts @@ -213,7 +213,7 @@ describe('Promise', () => { describe('retryable', () => { const retryOptions = utils.createRetryOptions(0, 0, 0, 0, 0, 0, 100); - const settings = {settings: {timeout: 0, retry: retryOptions}}; + const settings = {settings: {timeout: 0, retry: retryOptions, apiName: 'TestApi'}}; it('retries the API call', done => { let toAttempt = 3; @@ -371,7 +371,7 @@ describe('retryable', () => { assert.ok(err instanceof GoogleError); assert.strictEqual( err.message, - 'Retry total timeout exceeded 100 milliseconds before any response was received.' + 'Total timeout of API TestApi exceeded 100 milliseconds before any response was received.' ); assert.strictEqual(err!.code, status.DEADLINE_EXCEEDED); done(); From 36915361ad95c81d3fd45113377a26d0b2c9de75 Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Tue, 2 Jun 2020 14:40:22 -0700 Subject: [PATCH 7/8] lint --- src/normalCalls/retries.ts | 2 +- test/unit/apiCallable.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/normalCalls/retries.ts b/src/normalCalls/retries.ts index 293e6f24c..18bc1f3fb 100644 --- a/src/normalCalls/retries.ts +++ b/src/normalCalls/retries.ts @@ -46,7 +46,7 @@ export function retryable( func: GRPCCall, retry: RetryOptions, otherArgs: GRPCCallOtherArgs, - apiName?: string, + apiName?: string ): SimpleCallbackFunction { const delayMult = retry.backoffSettings.retryDelayMultiplier; const maxDelay = retry.backoffSettings.maxRetryDelayMillis; diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts index ecd58a45d..21e1d4b56 100644 --- a/test/unit/apiCallable.ts +++ b/test/unit/apiCallable.ts @@ -213,7 +213,9 @@ describe('Promise', () => { describe('retryable', () => { const retryOptions = utils.createRetryOptions(0, 0, 0, 0, 0, 0, 100); - const settings = {settings: {timeout: 0, retry: retryOptions, apiName: 'TestApi'}}; + const settings = { + settings: {timeout: 0, retry: retryOptions, apiName: 'TestApi'}, + }; it('retries the API call', done => { let toAttempt = 3; From f1a8b2692b928ec5b3901002a15faed051319758 Mon Sep 17 00:00:00 2001 From: xiaozhenliugg Date: Tue, 2 Jun 2020 15:17:51 -0700 Subject: [PATCH 8/8] add service name from construction --- src/gax.ts | 3 ++- test/unit/gax.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gax.ts b/src/gax.ts index 7fb05c68e..22a4a8e17 100644 --- a/src/gax.ts +++ b/src/gax.ts @@ -658,7 +658,7 @@ export function constructSettings( )! ); } - + const apiName = serviceName; defaults[jsName] = new CallSettings({ timeout, retry, @@ -666,6 +666,7 @@ export function constructSettings( ? createBundleOptions(bundlingConfig) : null, otherArgs, + apiName, }); } diff --git a/test/unit/gax.ts b/test/unit/gax.ts index 0ce57cdf9..3744dac12 100644 --- a/test/unit/gax.ts +++ b/test/unit/gax.ts @@ -110,6 +110,7 @@ describe('gax construct settings', () => { ); let settings = defaults.bundlingMethod; assert.strictEqual(settings.timeout, 40000); + assert.strictEqual(settings.apiName, SERVICE_NAME); expectRetryOptions(settings.retry); assert.deepStrictEqual(settings.retry.retryCodes, [1, 2]); assert.strictEqual(settings.otherArgs, otherArgs);