Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Commit a828ea6

Browse files
gaofanmichaelgaofanmichael
authored
feat: Enable isReady for debugger for GCF (#358)
This PR mainly serve for GCF. The introduced isReady will return a promise, and to be resolved either 1. Time since last listBreakpoint was within a heuristic time. 2. listBreakpoint completed successfully. 3. Debuggee registration expired or failed, listBreakpoint cannot be completed.
1 parent bbf4b98 commit a828ea6

File tree

3 files changed

+204
-17
lines changed

3 files changed

+204
-17
lines changed

‎src/agent/debuglet.ts‎

Lines changed: 129 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ const BREAKPOINT_ACTION_MESSAGE =
6363
'The only currently supported breakpoint actions' +
6464
' are CAPTURE and LOG.';
6565

66+
// PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS is a heuristic duration that we set
67+
// to force the debug agent to return a new promise for isReady. The value is
68+
// the average of Stackdriver debugger hanging get duration (40s) and TCP
69+
// time-out on GCF (540s).
70+
const PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS = (40 + 540) / 2 * 1000;
71+
6672
/**
6773
* Formats a breakpoint object prefixed with a provided message as a string
6874
* intended for logging.
@@ -107,6 +113,51 @@ const formatBreakpoints = function(
107113
.join('\n');
108114
};
109115

116+
/**
117+
* CachedPromise stores a promise. This promise can be resolved by calling
118+
* function resolve() and can only be resolved once.
119+
*/
120+
export class CachedPromise {
121+
private promiseResolve: (() => void) | null = null;
122+
private promise: Promise<void> = new Promise<void>((resolve) => {
123+
this.promiseResolve = resolve;
124+
});
125+
126+
get(): Promise<void> {
127+
return this.promise;
128+
}
129+
130+
resolve(): void {
131+
// Each promise can be resolved only once.
132+
if (this.promiseResolve) {
133+
this.promiseResolve();
134+
this.promiseResolve = null;
135+
}
136+
}
137+
}
138+
139+
/**
140+
* IsReady will return a promise to user after user starting the debug agent.
141+
* This promise will be resolved when one of the following is true:
142+
* 1. Time since last listBreakpoint was within a heuristic time.
143+
* 2. listBreakpoint completed successfully.
144+
* 3. Debuggee registration expired or failed, listBreakpoint cannot be
145+
* completed.
146+
*/
147+
export interface IsReady {
148+
isReady(): Promise<void>;
149+
}
150+
151+
/**
152+
* IsReadyManager is a wrapper class to use debuglet.isReady().
153+
*/
154+
class IsReadyImpl implements IsReady {
155+
constructor(private debuglet: Debuglet) {}
156+
isReady(): Promise<void> {
157+
return this.debuglet.isReady();
158+
}
159+
}
160+
110161
export class Debuglet extends EventEmitter {
111162
private debug: Debug;
112163
private v8debug: DebugApi|null;
@@ -115,6 +166,19 @@ export class Debuglet extends EventEmitter {
115166
private controller: Controller;
116167
private completedBreakpointMap: {[key: string]: boolean};
117168

169+
// breakpointFetchedTimestamp represents the last timestamp when
170+
// breakpointFetched was resolved, which means breakpoint update was
171+
// successful.
172+
private breakpointFetchedTimestamp: number;
173+
// breakpointFetched is a CachedPromise only to be resolved after breakpoint
174+
// fetch was successful. Its stored promise will be returned by isReady().
175+
private breakpointFetched: CachedPromise|null;
176+
// debuggeeRegistered is a CachedPromise only to be resolved after debuggee
177+
// registration was successful.
178+
private debuggeeRegistered: CachedPromise;
179+
180+
isReadyManager: IsReady = new IsReadyImpl(this);
181+
118182
// Exposed for testing
119183
config: DebugAgentConfig;
120184
fetcherActive: boolean;
@@ -175,6 +239,10 @@ export class Debuglet extends EventEmitter {
175239

176240
/** @private {Object.<string, Boolean>} */
177241
this.completedBreakpointMap = {};
242+
243+
this.breakpointFetched = null;
244+
this.breakpointFetchedTimestamp = -Infinity;
245+
this.debuggeeRegistered = new CachedPromise();
178246
}
179247

180248
static normalizeConfig_(config: DebugAgentConfig): DebugAgentConfig {
@@ -318,10 +386,31 @@ export class Debuglet extends EventEmitter {
318386
that.scheduleRegistration_(0 /* immediately */);
319387
that.emit('started');
320388
});
321-
322389
});
323390
}
324391

392+
/**
393+
* isReady returns a promise that only resolved if the last breakpoint update
394+
* happend within a duration (PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS). This
395+
* feature is mainly used in Google Cloud Function (GCF), as it is a
396+
* serverless environment and we wanted to make sure debug agent always
397+
* captures the snapshots.
398+
*/
399+
isReady(): Promise<void> {
400+
if (Date.now() < this.breakpointFetchedTimestamp +
401+
PROMISE_RESOLVE_CUT_OFF_IN_MILLISECONDS) {
402+
return Promise.resolve();
403+
} else {
404+
if (this.breakpointFetched) return this.breakpointFetched.get();
405+
this.breakpointFetched = new CachedPromise();
406+
this.debuggeeRegistered.get().then(() => {
407+
this.scheduleBreakpointFetch_(
408+
0 /*immediately*/, true /*only fetch once*/);
409+
});
410+
return this.breakpointFetched.get();
411+
}
412+
}
413+
325414
/**
326415
* @private
327416
*/
@@ -509,26 +598,32 @@ export class Debuglet extends EventEmitter {
509598
// TODO: Handle the case when `result` is undefined.
510599
that.emit(
511600
'registered', (result as {debuggee: Debuggee}).debuggee.id);
601+
that.debuggeeRegistered.resolve();
512602
if (!that.fetcherActive) {
513-
that.scheduleBreakpointFetch_(0);
603+
that.scheduleBreakpointFetch_(0, false);
514604
}
515605
});
516606
}, seconds * 1000).unref();
517607
}
518608

519609
/**
520610
* @param {number} seconds
611+
* @param {boolean} once
521612
* @private
522613
*/
523-
scheduleBreakpointFetch_(seconds: number): void {
614+
scheduleBreakpointFetch_(seconds: number, once: boolean): void {
524615
const that = this;
525-
526-
that.fetcherActive = true;
616+
if (!once) {
617+
that.fetcherActive = true;
618+
}
527619
setTimeout(function() {
528620
if (!that.running) {
529621
return;
530622
}
531-
assert(that.fetcherActive);
623+
624+
if (!once) {
625+
assert(that.fetcherActive);
626+
}
532627

533628
that.logger.info('Fetching breakpoints');
534629
// TODO: Address the case when `that.debuggee` is `null`.
@@ -541,18 +636,19 @@ export class Debuglet extends EventEmitter {
541636
// We back-off from fetching breakpoints, and try to register
542637
// again after a while. Successful registration will restart the
543638
// breakpoint fetcher.
639+
that.updatePromise();
544640
that.scheduleRegistration_(
545641
that.config.internal.registerDelayOnFetcherErrorSec);
546642
return;
547643
}
548-
549644
// TODO: Address the case where `response` is `undefined`.
550645
switch ((response as http.ServerResponse).statusCode) {
551646
case 404:
552647
// Registration expired. Deactivate the fetcher and queue
553648
// re-registration, which will re-active breakpoint fetching.
554649
that.logger.info('\t404 Registration expired.');
555650
that.fetcherActive = false;
651+
that.updatePromise();
556652
that.scheduleRegistration_(0 /*immediately*/);
557653
return;
558654

@@ -564,12 +660,12 @@ export class Debuglet extends EventEmitter {
564660
if (!body) {
565661
that.logger.error('\tinvalid list response: empty body');
566662
that.scheduleBreakpointFetch_(
567-
that.config.breakpointUpdateIntervalSec);
663+
that.config.breakpointUpdateIntervalSec, once);
568664
return;
569665
}
570666
if (body.waitExpired) {
571667
that.logger.info('\tLong poll completed.');
572-
that.scheduleBreakpointFetch_(0 /*immediately*/);
668+
that.scheduleBreakpointFetch_(0 /*immediately*/, once);
573669
return;
574670
}
575671
const bps = (body.breakpoints ||
@@ -591,14 +687,36 @@ export class Debuglet extends EventEmitter {
591687
that.logger.info(formatBreakpoints(
592688
'Active Breakpoints: ', that.activeBreakpointMap));
593689
}
594-
that.scheduleBreakpointFetch_(
595-
that.config.breakpointUpdateIntervalSec);
690+
that.breakpointFetchedTimestamp = Date.now();
691+
if (once) {
692+
if (that.breakpointFetched) {
693+
that.breakpointFetched.resolve();
694+
that.breakpointFetched = null;
695+
}
696+
} else {
697+
that.scheduleBreakpointFetch_(
698+
that.config.breakpointUpdateIntervalSec, once);
699+
}
596700
return;
597701
}
598702
});
599703
}, seconds * 1000).unref();
600704
}
601705

706+
/**
707+
* updatePromise_ is called when debuggee is expired. debuggeeRegistered
708+
* CachedPromise will be refreshed. Also, breakpointFetched CachedPromise will
709+
* be resolved so that uses (such as GCF users) will not hang forever to wait
710+
* non-fetchable breakpoints.
711+
*/
712+
private updatePromise() {
713+
this.debuggeeRegistered = new CachedPromise();
714+
if (this.breakpointFetched) {
715+
this.breakpointFetched.resolve();
716+
this.breakpointFetched = null;
717+
}
718+
}
719+
602720
/**
603721
* Given a list of server breakpoints, update our internal list of breakpoints
604722
* @param {Array.<Breakpoint>} breakpoints
@@ -612,7 +730,6 @@ export class Debuglet extends EventEmitter {
612730
that.logger.info(
613731
formatBreakpoints('Server breakpoints: ', updatedBreakpointMap));
614732
}
615-
616733
breakpoints.forEach(function(breakpoint: stackdriver.Breakpoint) {
617734

618735
// TODO: Address the case when `breakpoint.id` is `undefined`.

‎src/index.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
import {DebugAgentConfig, StackdriverConfig} from './agent/config';
18-
import {Debuglet} from './agent/debuglet';
18+
import {Debuglet, IsReady} from './agent/debuglet';
1919
import {Debug} from './client/stackdriver/debug';
2020

2121
const pjson = require('../../package.json');
@@ -36,7 +36,7 @@ let debuglet: Debuglet;
3636
* debug.startAgent();
3737
*/
3838
export function start(options: DebugAgentConfig|StackdriverConfig): Debuglet|
39-
undefined {
39+
IsReady {
4040
options = options || {};
4141
const agentConfig: DebugAgentConfig =
4242
(options as StackdriverConfig).debug || (options as DebugAgentConfig);
@@ -50,6 +50,5 @@ export function start(options: DebugAgentConfig|StackdriverConfig): Debuglet|
5050
debuglet = new Debuglet(debug, agentConfig);
5151
debuglet.start();
5252

53-
// We return the debuglet to facilitate testing.
54-
return agentConfig.testMode_ ? debuglet : undefined;
53+
return agentConfig.testMode_ ? debuglet : debuglet.isReadyManager;
5554
}

‎test/test-debuglet.ts‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import * as stackdriver from '../src/types/stackdriver';
2626

2727
(DEFAULT_CONFIG as any).allowExpressions = true;
2828
(DEFAULT_CONFIG as any).workingDirectory = path.join(__dirname, '..', '..');
29-
import {Debuglet} from '../src/agent/debuglet';
29+
import {Debuglet, CachedPromise} from '../src/agent/debuglet';
3030
import * as dns from 'dns';
3131
import * as extend from 'extend';
3232
const metadata: {project: any, instance: any} = require('gcp-metadata');
@@ -77,6 +77,17 @@ function verifyBreakpointRejection(re: RegExp, body: {breakpoint: any}) {
7777
return status.isError && hasCorrectDescription;
7878
}
7979

80+
describe('CachedPromise', function() {
81+
it('CachedPromise.get() will resolve after CachedPromise.resolve()', (done) => {
82+
this.timeout(2000);
83+
let cachedPromise = new CachedPromise();
84+
cachedPromise.get().then(() => {
85+
done();
86+
});
87+
cachedPromise.resolve();
88+
});
89+
});
90+
8091
describe('Debuglet', function() {
8192
describe('runningOnGCP', () => {
8293
// TODO: Make this more precise.
@@ -952,6 +963,66 @@ describe('Debuglet', function() {
952963
debuglet.start();
953964
});
954965

966+
it('should have breakpoints fetched when promise is resolved',
967+
function(done) {
968+
this.timeout(2000);
969+
const breakpoint: stackdriver.Breakpoint = {
970+
id: 'test1',
971+
action: 'CAPTURE',
972+
location: {path: 'build/test/fixtures/foo.js', line: 2}
973+
} as stackdriver.Breakpoint;
974+
975+
const debug = new Debug(
976+
{projectId: 'fake-project', credentials: fakeCredentials},
977+
packageInfo);
978+
const debuglet = new Debuglet(debug, defaultConfig);
979+
980+
const scope = nock(API)
981+
.post(REGISTER_PATH)
982+
.reply(200, {debuggee: {id: DEBUGGEE_ID}})
983+
.get(BPS_PATH + '?successOnTimeout=true')
984+
.twice()
985+
.reply(200, {breakpoints: [breakpoint]});
986+
const debugPromise = debuglet.isReadyManager.isReady();
987+
debuglet.once('registered', function reg(id: string) {
988+
debugPromise.then(() => {
989+
// Once debugPromise is resolved, debuggee must be registered.
990+
assert(debuglet.debuggee);
991+
setTimeout(function() {
992+
assert.deepEqual(debuglet.activeBreakpointMap.test1, breakpoint);
993+
debuglet.activeBreakpointMap = {};
994+
debuglet.stop();
995+
scope.done();
996+
done();
997+
}, 1000);
998+
});
999+
});
1000+
debuglet.start();
1001+
});
1002+
1003+
it('should resolve breakpointFetched promise when registration expires',
1004+
function(done) {
1005+
this.timeout(2000);
1006+
const debug = new Debug(
1007+
{projectId: 'fake-project', credentials: fakeCredentials},
1008+
packageInfo);
1009+
const debuglet = new Debuglet(debug, defaultConfig);
1010+
1011+
const scope = nock(API)
1012+
.post(REGISTER_PATH)
1013+
.reply(200, {debuggee: {id: DEBUGGEE_ID}})
1014+
.get(BPS_PATH + '?successOnTimeout=true')
1015+
.reply(404); // signal re-registration.
1016+
const debugPromise = debuglet.isReadyManager.isReady();
1017+
debugPromise.then(() => {
1018+
debuglet.stop();
1019+
scope.done();
1020+
done();
1021+
});
1022+
1023+
debuglet.start();
1024+
});
1025+
9551026
it('should reject breakpoints with conditions when allowExpressions=false',
9561027
function(done) {
9571028
this.timeout(2000);

0 commit comments

Comments
 (0)