diff --git a/ts/src/config.ts b/ts/src/config.ts index 403d8199..371ff931 100644 --- a/ts/src/config.ts +++ b/ts/src/config.ts @@ -16,6 +16,7 @@ import {AuthenticationConfig, Common, ServiceConfig} from '../third_party/types/common-types'; +const parseDuration: (str: string) => number = require('parse-duration'); const common: Common = require('@google-cloud/common'); const extend = require('extend'); @@ -77,9 +78,25 @@ export interface Config extends AuthenticationConfig { // stack depth may increase overhead of profiling. heapMaxStackDepth?: number; - // Time to wait before trying to create a profile again if profile creation - // fails. - backoffMillis?: number; + // On first error during profile creation, if the backoff is not specified + // by the server response, then profiler will between 0 and + // initialBackoffMillis before asking the server to create a profile again. + // After a successful profile creation, the backoff will be reset to + // initialExpBackoffMillis. + initialBackoffMillis?: number; + + // If the backoff is not specified by the server response, then profiler will + // wait at most maxBackoffMillis before asking server to create a profile + // again. + maxBackoffMillis?: number; + + // On each consecutive error in profile creation, the maximum backoff will + // increase by this factor. The backoff will be random value selected + // from a uniform distribution between 0 and the maximum backoff. + backoffMultiplier?: number; + + // Server-specified backoffs will be capped at backoffLimitMillis. + backoffLimitMillis?: number; } // Interface for an initialized config. @@ -94,7 +111,10 @@ export interface ProfilerConfig extends AuthenticationConfig { timeIntervalMicros: number; heapIntervalBytes: number; heapMaxStackDepth: number; - backoffMillis: number; + initialBackoffMillis: number; + maxBackoffMillis: number; + backoffMultiplier: number; + backoffLimitMillis: number; } // Default values for configuration for a profiler. @@ -106,5 +126,8 @@ export const defaultConfig = { timeIntervalMicros: 1000, heapIntervalBytes: 512 * 1024, heapMaxStackDepth: 64, - backoffMillis: 5 * 60 * 1000 + initialBackoffMillis: 1000, + maxBackoffMillis: parseDuration('1h'), + backoffMultiplier: 1.3, + backoffLimitMillis: parseDuration('7d'), // 7 days }; diff --git a/ts/src/profiler.ts b/ts/src/profiler.ts index 09d56d6a..80c4be8c 100644 --- a/ts/src/profiler.ts +++ b/ts/src/profiler.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import * as http from 'http'; import * as path from 'path'; import * as pify from 'pify'; import * as zlib from 'zlib'; @@ -72,6 +73,21 @@ export interface RequestProfile { labels?: {instance?: string}; } +interface ServerBackoffResponse { + statusMessage: string; + body: {details: {retryDelay: string}}; +} + +/** + * Returns true if response indicates a backoff. + */ +// tslint:disable-next-line: no-any +function isServerBackoffResponse(response: any): + response is ServerBackoffResponse { + return response.body && response.body.details && + typeof response.body.details.retryDelay === 'string'; +} + /** * @return true if an deployment is a Deployment and false otherwise. */ @@ -102,7 +118,7 @@ function isRequestProfile(prof: any): prof is RequestProfile { } /** - * Returns true if response has statusCode. + * @return - true if response has statusCode and false otherwise. */ // tslint:disable-next-line: no-any function hasHttpStatusCode(response: any): @@ -125,6 +141,50 @@ async function profileBytes(p: perftools.profiles.IProfile): Promise { return gzBuf.toString('base64'); } +/** + * Error constructed from http server response which indicates backoff. + */ +class BackoffResponseError extends Error { + backoffMillis: number; + constructor(response: ServerBackoffResponse) { + super(response.statusMessage); + this.backoffMillis = parseDuration(response.body.details.retryDelay); + } +} + +/** + * @return - true if error is a BackoffResponseError and false otherwise + */ +function isBackoffResponseError(err: Error): err is BackoffResponseError { + // tslint:disable-next-line: no-any + return typeof (err as any).backoffMillis === 'number'; +} + +/** + * Class which tracks how long to wait before the next retry and can be + * used to get this backoff. + */ +export class Retryer { + private nextMaxBackoffMillis: number; + constructor( + readonly initialBackoffMillis: number, readonly maxBackoffMillis: number, + readonly backoffMultiplier: number) { + this.nextMaxBackoffMillis = this.initialBackoffMillis; + } + getBackoff(): number { + const curBackoff = Math.random() * this.nextMaxBackoffMillis; + this.nextMaxBackoffMillis = + this.backoffMultiplier * this.nextMaxBackoffMillis; + if (this.nextMaxBackoffMillis > this.maxBackoffMillis) { + this.nextMaxBackoffMillis = this.maxBackoffMillis; + } + return curBackoff; + } + reset() { + this.nextMaxBackoffMillis = this.initialBackoffMillis; + } +} + /** * Polls profiler server for instructions on behalf of a task and * collects and uploads profiles as requested @@ -135,6 +195,7 @@ export class Profiler extends common.ServiceObject { private profileLabels: {instance?: string}; private deployment: Deployment; private profileTypes: string[]; + private retryer: Retryer; // Public for testing. timeProfiler: TimeProfiler|undefined; @@ -184,6 +245,10 @@ export class Profiler extends common.ServiceObject { this.heapProfiler = new HeapProfiler( this.config.heapIntervalBytes, this.config.heapMaxStackDepth); } + + this.retryer = new Retryer( + this.config.initialBackoffMillis, this.config.maxBackoffMillis, + this.config.backoffMultiplier); } /** @@ -206,8 +271,6 @@ export class Profiler extends common.ServiceObject { */ async runLoop() { const delayMillis = await this.collectProfile(); - - // Schedule the next profile. setTimeout(this.runLoop.bind(this), delayMillis).unref(); } @@ -216,12 +279,7 @@ export class Profiler extends common.ServiceObject { * a profile and uploads it. * * @return - time, in ms, to wait before asking profiler server again about - * collecting another profile. - * - * TODO: implement backoff and retry. When error encountered in - * createProfile() should be retried when response indicates this request - * should be retried or with exponential backoff (up to one hour) if the - * response does not indicate when to retry this request. + * collecting another profile, or retryer specifying exponential backoff. */ async collectProfile(): Promise { let prof: RequestProfile; @@ -229,8 +287,12 @@ export class Profiler extends common.ServiceObject { prof = await this.createProfile(); } catch (err) { this.logger.error(`Failed to create profile: ${err}`); - return this.config.backoffMillis; + if (isBackoffResponseError(err)) { + return Math.min(err.backoffMillis, this.config.backoffLimitMillis); + } + return this.retryer.getBackoff(); } + this.retryer.reset(); try { await this.profileAndUpload(prof); } catch (err) { @@ -268,25 +330,29 @@ export class Profiler extends common.ServiceObject { body: reqBody, json: true, }; - - this.logger.debug(`Attempting to create profile.`); - const [prof, response] = await this.request(options); - if (!hasHttpStatusCode(response)) { - throw new Error('Server response missing status information.'); - } - if (isErrorResponseStatusCode(response.statusCode)) { - let message: number|string = response.statusCode; - // tslint:disable-next-line: no-any - if ((response as any).statusMessage) { - message = response.statusMessage; - } - throw new Error(message.toString()); - } - if (!isRequestProfile(prof)) { - throw new Error(`Profile not valid: ${JSON.stringify(prof)}.`); - } - this.logger.debug(`Successfully created profile ${prof.profileType}.`); - return prof; + return new Promise((resolve, reject) => { + this.request( + options, + (err: Error, prof: object, response: http.ServerResponse) => { + if (response && isErrorResponseStatusCode(response.statusCode)) { + if (isServerBackoffResponse(response)) { + throw new BackoffResponseError(response); + } + throw new Error(response.statusMessage); + } + if (err) { + reject(err); + return; + } + if (isRequestProfile(prof)) { + this.logger.debug( + `Successfully created profile ${prof.profileType}.`); + resolve(prof); + return; + } + throw new Error(`Profile not valid: ${JSON.stringify(prof)}.`); + }); + }); } /** diff --git a/ts/test/test-init-config.ts b/ts/test/test-init-config.ts index d2284698..7199ab0b 100644 --- a/ts/test/test-init-config.ts +++ b/ts/test/test-init-config.ts @@ -15,6 +15,7 @@ */ import * as assert from 'assert'; +import * as extend from 'extend'; import * as gcpMetadata from 'gcp-metadata'; import * as sinon from 'sinon'; @@ -42,6 +43,16 @@ describe('initConfig', () => { process.env = savedEnv; }); + const internalConfigParams = { + timeIntervalMicros: 1000, + heapIntervalBytes: 512 * 1024, + heapMaxStackDepth: 64, + initialBackoffMillis: 1000, + maxBackoffMillis: 60 * 60 * 1000, + backoffMultiplier: 1.3, + backoffLimitMillis: 7 * 24 * 60 * 60 * 1000 + }; + it('should not modify specified fields when not on GCE', async () => { metadataStub = sinon.stub(gcpMetadata, 'instance') .throwsException('cannot access metadata'); @@ -55,21 +66,8 @@ describe('initConfig', () => { zone: 'zone', projectId: 'fake-projectId' }; - const expConfig = { - logLevel: 2, - serviceContext: {version: 'fake-version', service: 'fake-service'}, - disableHeap: true, - disableTime: true, - instance: 'instance', - zone: 'zone', - projectId: 'fake-projectId', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 - }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual(initializedConfig, extend(config, internalConfigParams)); }); it('should not modify specified fields when on GCE', async () => { @@ -89,21 +87,8 @@ describe('initConfig', () => { zone: 'zone', projectId: 'fake-projectId' }; - const expConfig = { - logLevel: 2, - serviceContext: {version: 'fake-version', service: 'fake-service'}, - disableHeap: true, - disableTime: true, - instance: 'instance', - zone: 'zone', - projectId: 'fake-projectId', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 - }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual(initializedConfig, extend(config, internalConfigParams)); }); it('should get zone and instance from GCE', async () => { @@ -128,14 +113,11 @@ describe('initConfig', () => { disableTime: true, instance: 'gce-instance', zone: 'gce-zone', - projectId: 'projectId', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 + projectId: 'projectId' }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual( + initializedConfig, extend(expConfig, internalConfigParams)); }); it('should not reject when not on GCE and no zone and instance found', @@ -152,13 +134,10 @@ describe('initConfig', () => { disableHeap: false, disableTime: false, projectId: 'fake-projectId', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual( + initializedConfig, extend(expConfig, internalConfigParams)); }); it('should reject when no service specified', () => { @@ -192,20 +171,8 @@ describe('initConfig', () => { instance: 'instance', zone: 'zone' }; - const expConfig = { - logLevel: 2, - serviceContext: {version: '', service: 'fake-service'}, - disableHeap: true, - disableTime: true, - instance: 'instance', - zone: 'zone', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 - }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual(initializedConfig, extend(config, internalConfigParams)); }); it('should get values from from environment variable when not specified in config or environment variables', @@ -231,14 +198,11 @@ describe('initConfig', () => { disableHeap: true, disableTime: true, instance: 'envConfig-instance', - zone: 'envConfig-zone', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 + zone: 'envConfig-zone' }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual( + initializedConfig, extend(expConfig, internalConfigParams)); }); it('should not get values from from environment variable when values specified in config', @@ -265,21 +229,9 @@ describe('initConfig', () => { instance: 'instance', zone: 'zone' }; - const expConfig = { - projectId: 'config-projectId', - logLevel: 1, - serviceContext: {version: 'config-version', service: 'config-service'}, - disableHeap: false, - disableTime: false, - instance: 'instance', - zone: 'zone', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 - }; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual( + initializedConfig, extend(config, internalConfigParams)); }); it('should get values from from environment config when not specified in config or other environment variables', @@ -297,15 +249,12 @@ describe('initConfig', () => { disableTime: true, instance: 'envConfig-instance', zone: 'envConfig-zone', - projectId: 'envConfig-fake-projectId', - timeIntervalMicros: 1000, - heapIntervalBytes: 512 * 1024, - heapMaxStackDepth: 64, - backoffMillis: 300000 + projectId: 'envConfig-fake-projectId' }; const config = {}; const initializedConfig = await initConfig(config); - assert.deepEqual(initializedConfig, expConfig); + assert.deepEqual( + initializedConfig, extend(expConfig, internalConfigParams)); }); }); diff --git a/ts/test/test-profiler.ts b/ts/test/test-profiler.ts index 2052ba6d..3654b1a0 100644 --- a/ts/test/test-profiler.ts +++ b/ts/test/test-profiler.ts @@ -24,7 +24,7 @@ import * as zlib from 'zlib'; import {perftools} from '../../proto/profile'; import {ProfilerConfig} from '../src/config'; -import {Profiler} from '../src/profiler'; +import {Profiler, Retryer} from '../src/profiler'; import {HeapProfiler} from '../src/profilers/heap-profiler'; import {TimeProfiler} from '../src/profilers/time-profiler'; import {Common} from '../third_party/types/common-types'; @@ -33,6 +33,7 @@ import {decodedHeapProfile, decodedTimeProfile, heapProfile, timeProfile} from ' const common: Common = require('@google-cloud/common'); const v8TimeProfiler = require('bindings')('time_profiler'); +const parseDuration: (str: string) => number = require('parse-duration'); const fakeCredentials = require('../../ts/test/fixtures/gcloud-credentials.json'); @@ -49,7 +50,10 @@ const testConfig: ProfilerConfig = { timeIntervalMicros: 1000, heapIntervalBytes: 512 * 1024, heapMaxStackDepth: 64, - backoffMillis: 1000 + initialBackoffMillis: 1000, + maxBackoffMillis: parseDuration('1h'), + backoffMultiplier: 1.3, + backoffLimitMillis: parseDuration('7d') }; const API = 'https://cloudprofiler.googleapis.com/v2'; @@ -78,6 +82,31 @@ function nockOauth2(): nock.Scope { }); } +describe('Retryer', () => { + let randomStub: sinon.SinonStub|undefined; + before(() => { + randomStub = sinon.stub(Math, 'random').returns(0.5); + }); + after(() => { + if (randomStub) { + randomStub.restore(); + } + }); + it('should backoff until max-backoff reached', () => { + const retryer = new Retryer(1000, 1000000, 5); + assert.equal(retryer.getBackoff(), 0.5 * 1000); + assert.equal(retryer.getBackoff(), 0.5 * 5000); + assert.equal(retryer.getBackoff(), 0.5 * 25000); + assert.equal(retryer.getBackoff(), 0.5 * 125000); + assert.equal(retryer.getBackoff(), 0.5 * 625000); + assert.equal(retryer.getBackoff(), 0.5 * 1000000); + assert.equal(retryer.getBackoff(), 0.5 * 1000000); + assert.equal(retryer.getBackoff(), 0.5 * 1000000); + assert.equal(retryer.getBackoff(), 0.5 * 1000000); + assert.equal(retryer.getBackoff(), 0.5 * 1000000); + }); +}); + describe('Profiler', () => { afterEach(() => { nock.cleanAll(); @@ -369,7 +398,12 @@ describe('Profiler', () => { name: 'projects/12345678901/test-projectId', profileType: 'WALL', duration: '10s', - labels: {instance: config.instance} + labels: {instance: config.instance}, + deployment: { + projectId: 'test-projectId', + target: 'target', + labels: {zone: 'zone', version: 'version'}, + } }; nockOauth2(); const requestProfileMock = @@ -394,11 +428,10 @@ describe('Profiler', () => { profileType: 'WALL', duration: '10s', }; - requestStub = sinon.stub(common.ServiceObject.prototype, 'request') - .onCall(0) - .returns(new Promise(resolve => { - resolve([response, {statusCode: 200}]); - })); + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith(1, undefined, response, {statusCode: 200}); const expRequestBody = { deployment: { labels: {version: 'test-version'}, @@ -423,11 +456,10 @@ describe('Profiler', () => { profileType: 'WALL', duration: '10s', }; - requestStub = sinon.stub(common.ServiceObject.prototype, 'request') - .onCall(0) - .returns(new Promise(resolve => { - resolve([response, {statusCode: 200}]); - })); + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith(1, undefined, response, {statusCode: 200}); const expRequestBody = { deployment: { labels: {version: 'test-version'}, @@ -470,9 +502,11 @@ describe('Profiler', () => { duration: '10s', labels: {instance: config.instance} }; - requestStub = sinon.stub(common.ServiceObject.prototype, 'request') - .onCall(0) - .returns(Promise.reject(new Error('Network error'))); + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith( + 1, new Error('Network error'), undefined, undefined); const profiler = new Profiler(testConfig); try { await profiler.createProfile(); @@ -493,11 +527,9 @@ describe('Profiler', () => { requestStub = sinon.stub(common.ServiceObject.prototype, 'request') .onCall(0) - .returns(new Promise(resolve => { - resolve([ - {}, {statusCode: 500, statusMessage: '500 status code'} - ]); - })); + .callsArgWith( + 1, undefined, undefined, + {statusCode: 500, statusMessage: '500 status code'}); const profiler = new Profiler(testConfig); try { @@ -507,37 +539,48 @@ describe('Profiler', () => { assert.equal(err.message, '500 status code'); } }); - it('should throw status code when response has non-200 status and no status message.', + it('should throw error with server-specified backoff when non-200 error' + + ' and backoff specified', async () => { const config = extend(true, {}, testConfig); - const response = { + const requestProfileResponseBody = { name: 'projects/12345678901/test-projectId', profileType: 'WALL', duration: '10s', labels: {instance: config.instance} }; - requestStub = sinon.stub(common.ServiceObject.prototype, 'request') - .onCall(0) - .returns(new Promise(resolve => { - resolve([{}, {statusCode: 500}]); - })); + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith( + 1, undefined, undefined, + {statusCode: 409, body: {details: {retryDelay: '50s'}}}); const profiler = new Profiler(testConfig); try { await profiler.createProfile(); assert.fail('expected error, no error thrown'); } catch (err) { - assert.equal(err.message, '500'); + assert.equal(err.backoffMillis, 50000); } }); }); describe('collectProfile', () => { let requestStub: undefined|sinon.SinonStub; + let randomStub: sinon.SinonStub|undefined; + before(() => { + randomStub = sinon.stub(Math, 'random').returns(0.5); + }); afterEach(() => { if (requestStub) { requestStub.restore(); } }); + after(() => { + if (randomStub) { + randomStub.restore(); + } + }); it('should indicate collectProfile should be called immediately when no errors', async () => { const config = extend(true, {}, testConfig); @@ -550,13 +593,11 @@ describe('Profiler', () => { requestStub = sinon.stub(common.ServiceObject.prototype, 'request') .onCall(0) - .returns(new Promise(resolve => { - resolve([requestProfileResponseBody, {statusCode: 200}]); - })) + .callsArgWith( + 1, undefined, requestProfileResponseBody, + {statusCode: 200}) .onCall(1) - .returns(new Promise(resolve => { - resolve([{}, {statusCode: 200}]); - })); + .callsArgWith(1, undefined, undefined, {statusCode: 200}); const profiler = new Profiler(testConfig); @@ -565,8 +606,8 @@ describe('Profiler', () => { assert.equal( 0, delayMillis, 'No delay before asking to collect next profile'); }); - it('should indicate collectProfile should be called after some backoff' + - 'when error in requesting profile', + it('should return expect backoff when non-200 response and no backoff' + + ' indicated', async () => { const config = extend(true, {}, testConfig); const requestProfileResponseBody = { @@ -575,22 +616,92 @@ describe('Profiler', () => { duration: '10s', labels: {instance: config.instance} }; - requestStub = sinon.stub(common.ServiceObject.prototype, 'request') - .onCall(0) - .returns(new Promise(resolve => { - resolve([{}, {statusCode: 404}]); - })); + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith(1, undefined, undefined, {statusCode: 404}); + const profiler = new Profiler(testConfig); + profiler.timeProfiler = instance(mockTimeProfiler); + const delayMillis = await profiler.collectProfile(); + assert.deepEqual(500, delayMillis); + }); + it('should reset backoff after success', async () => { + const config = extend(true, {}, testConfig); + const requestProfileResponseBody = { + name: 'projects/12345678901/test-projectId', + profileType: 'WALL', + duration: '10s', + labels: {instance: config.instance} + }; + + const createProfileResponseBody = { + name: 'projects/12345678901/test-projectId', + profileType: 'WALL', + duration: '10s', + labels: {instance: config.instance} + }; + requestStub = + sinon + .stub(common.ServiceObject.prototype, 'request') + // createProfile - first failure + .onCall(0) + .callsArgWith(1, undefined, undefined, {statusCode: 404}) + // createProfile - second failure + .onCall(1) + .callsArgWith(1, undefined, undefined, {statusCode: 404}) + // createProfile - third failure + .onCall(2) + .callsArgWith(1, undefined, undefined, {statusCode: 404}) + // createProfile + .onCall(3) + // createProfile - success + .callsArgWith( + 1, undefined, createProfileResponseBody, {statusCode: 200}) + // upload profiler - success + .onCall(4) + .callsArgWith(1, undefined, undefined, {statusCode: 200}) + // createProfile - failure + .onCall(5) + .callsArgWith( + 1, new Error('error creating profile'), undefined, undefined); + const profiler = new Profiler(config); + profiler.timeProfiler = instance(mockTimeProfiler); + let delayMillis = await profiler.collectProfile(); + assert.deepEqual(500, delayMillis); + delayMillis = await profiler.collectProfile(); + assert.deepEqual(650, delayMillis); + delayMillis = await profiler.collectProfile(); + assert.deepEqual(845, delayMillis); + delayMillis = await profiler.collectProfile(); + assert.deepEqual(0, delayMillis); + delayMillis = await profiler.collectProfile(); + assert.deepEqual(500, delayMillis); + }); + it('should return server-specified backoff when non-200 error and backoff' + + ' specified', + async () => { + const config = extend(true, {}, testConfig); + const requestProfileResponseBody = { + name: 'projects/12345678901/test-projectId', + profileType: 'WALL', + duration: '10s', + labels: {instance: config.instance} + }; + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith( + 1, undefined, undefined, + {statusCode: 409, body: {details: {retryDelay: '50s'}}}); const profiler = new Profiler(testConfig); profiler.timeProfiler = instance(mockTimeProfiler); const delayMillis = await profiler.collectProfile(); - assert.equal( - 1000, delayMillis, - 'No delay before asking to collect next profile'); + assert.equal(50000, delayMillis); }); - it('should indicate collectProfile should be called immediately error' + - ' in collecting and uploading profile.', + it('should return backoff limit, when server specified backoff is greater' + + 'then backoff limit', async () => { const config = extend(true, {}, testConfig); const requestProfileResponseBody = { @@ -602,16 +713,38 @@ describe('Profiler', () => { requestStub = sinon.stub(common.ServiceObject.prototype, 'request') .onCall(0) - .returns(new Promise(resolve => { - resolve([requestProfileResponseBody, {statusCode: 200}]); - })) + .callsArgWith( + 1, undefined, undefined, + {statusCode: 409, body: {details: {retryDelay: '1000h'}}}); + + const profiler = new Profiler(testConfig); + profiler.timeProfiler = instance(mockTimeProfiler); + const delayMillis = await profiler.collectProfile(); + assert.equal(parseDuration('7d'), delayMillis); + }); + it('should indicate collectProfile should be called immediately if there' + + ' is an error when collecting and uploading profile.', + async () => { + const config = extend(true, {}, testConfig); + const createProfileResponseBody = { + name: 'projects/12345678901/test-projectId', + profileType: 'WALL', + duration: '10s', + labels: {instance: config.instance} + }; + requestStub = + sinon.stub(common.ServiceObject.prototype, 'request') + .onCall(0) + .callsArgWith( + 1, undefined, createProfileResponseBody, {statusCode: 200}) .onCall(1) - .returns(Promise.reject('Error uploading profile')); + .callsArgWith( + 1, new Error('Error uploading'), undefined, undefined); + const profiler = new Profiler(testConfig); profiler.timeProfiler = instance(mockTimeProfiler); const delayMillis = await profiler.collectProfile(); - assert.equal( - 0, delayMillis, 'No delay before asking to collect next profile'); + assert.equal(0, delayMillis); }); }); }); diff --git a/ts/third_party/types/README.google b/ts/third_party/types/README.google index fdf26773..bc280dde 100644 --- a/ts/third_party/types/README.google +++ b/ts/third_party/types/README.google @@ -8,5 +8,6 @@ Description: Local Modifications: All code except common-types.ts has been removed. RequestOptions interface added. - ServiceObject interface modified so promisified request can be used. + ServiceObject interface modified so both non-promisified and promisified + request can be used. Definitions modified to comply with gts 0.5 diff --git a/ts/third_party/types/common-types.ts b/ts/third_party/types/common-types.ts index 39f1bf6c..e86b19d8 100644 --- a/ts/third_party/types/common-types.ts +++ b/ts/third_party/types/common-types.ts @@ -94,7 +94,10 @@ export interface RequestOptions { export interface ServiceObject { new(config: ServiceObjectConfig): ServiceObject; // TODO: Determine if this signature is correct. - request: (reqOpts: RequestOptions) => Promise; + request: + ((reqOpts: RequestOptions, + callback: (err: Error, body: object, response: http.ServerResponse) => + void) => void)&((reqOpts: RequestOptions) => Promise); } export interface Common {