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

Commit 225e7db

Browse files
ofrobotsofrobots
authored
Refactor debuggee state out of controller and make Controller a ServiceObject (#195)
* Remove debuggee state from controller class * make Controller a ServiceObject * Try to run system and e2e tests on developer builds as well
1 parent a807998 commit 225e7db

File tree

6 files changed

+78
-60
lines changed

6 files changed

+78
-60
lines changed

‎bin/run-test.sh‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,16 @@ if [ "$cover" ]; then
4343
rm -rf ./coverage
4444
fi
4545

46-
if [ "${TRAVIS_PULL_REQUEST}" = "false" ]
46+
if [ -z "${TRAVIS_PULL_REQUEST}" ] || [ "${TRAVIS_PULL_REQUEST}" = "false" ]
4747
then
48-
npm run system-test
49-
./bin/run-e2e.sh || exit 1
48+
if [ -z "${GCLOUD_PROJECT}"]; then
49+
echo "============================================================"
50+
echo "Unable to run system and e2e tests. Provide valid project id"
51+
echo "via GCLOUD_PROJECT and ensure auth credentials are available"
52+
echo "============================================================"
53+
exit 1
54+
else
55+
npm run system-test
56+
./bin/run-e2e.sh || exit 1
57+
fi
5058
fi

‎src/agent/debuglet.js‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,17 @@ Debuglet.prototype.scheduleBreakpointFetch_ = function(seconds) {
267267
assert(that.fetcherActive_);
268268

269269
that.logger_.info('Fetching breakpoints');
270-
that.debugletApi_.listBreakpoints(function(err, response, body) {
270+
that.debugletApi_.listBreakpoints(that.debuggee_, function(err, response,
271+
body) {
271272
if (err) {
272273
that.logger_.error('Unable to fetch breakpoints – stopping fetcher',
273-
err);
274+
err);
274275
that.fetcherActive_ = false;
275-
// We back-off from fetching breakpoints, and try to register again after
276-
// a while. Successful registration will restart the breakpoint fetcher.
276+
// We back-off from fetching breakpoints, and try to register again
277+
// after a while. Successful registration will restart the breakpoint
278+
// fetcher.
277279
that.scheduleRegistration_(
278-
that.config_.internal.registerDelayOnFetcherErrorSec);
280+
that.config_.internal.registerDelayOnFetcherErrorSec);
279281
return;
280282
}
281283

@@ -444,14 +446,15 @@ Debuglet.prototype.completeBreakpoint_ = function(breakpoint) {
444446
var that = this;
445447

446448
that.logger_.info('\tupdating breakpoint data on server', breakpoint.id);
447-
that.debugletApi_.updateBreakpoint(breakpoint, function(err/*, body*/) {
448-
if (err) {
449-
that.logger_.error('Unable to complete breakpoint on server', err);
450-
} else {
451-
that.completedBreakpointMap_[breakpoint.id] = true;
452-
that.removeBreakpoint_(breakpoint);
453-
}
454-
});
449+
that.debugletApi_.updateBreakpoint(
450+
breakpoint, that.debuggee_, function(err /*, body*/) {
451+
if (err) {
452+
that.logger_.error('Unable to complete breakpoint on server', err);
453+
} else {
454+
that.completedBreakpointMap_[breakpoint.id] = true;
455+
that.removeBreakpoint_(breakpoint);
456+
}
457+
});
455458
};
456459

457460
/**
@@ -462,11 +465,12 @@ Debuglet.prototype.completeBreakpoint_ = function(breakpoint) {
462465
Debuglet.prototype.rejectBreakpoint_ = function(breakpoint) {
463466
var that = this;
464467

465-
that.debugletApi_.updateBreakpoint(breakpoint, function(err/*, body*/) {
466-
if (err) {
467-
that.logger_.error('Unable to complete breakpoint on server', err);
468-
}
469-
});
468+
that.debugletApi_.updateBreakpoint(
469+
breakpoint, that.debuggee_, function(err /*, body*/) {
470+
if (err) {
471+
that.logger_.error('Unable to complete breakpoint on server', err);
472+
}
473+
});
470474
};
471475

472476
/**

‎src/controller.js‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
*/
2222

2323
var assert = require('assert');
24+
var common = require('@google-cloud/common');
2425
var qs = require('querystring');
26+
var util = require('util');
2527

2628
/** @const {string} Cloud Debug API endpoint */
2729
var API = 'https://clouddebugger.googleapis.com/v2/controller';
@@ -30,32 +32,31 @@ var API = 'https://clouddebugger.googleapis.com/v2/controller';
3032
* @constructor
3133
*/
3234
function Controller(debug) {
33-
/** @priavate {Debug} */
34-
this.debug_ = debug;
35-
36-
/** @private {string} debuggee id provided by the server once registered */
37-
this.debuggeeId_ = null;
35+
common.ServiceObject.call(this, {
36+
parent: debug,
37+
baseUrl: '/controller'
38+
});
3839

3940
/** @private {string} */
4041
this.nextWaitToken_ = null;
4142
}
4243

44+
util.inherits(Controller, common.ServiceObject);
45+
4346
/**
4447
* Register to the API (implementation)
4548
*
4649
* @param {!function(?Error,Object=)} callback
4750
* @private
4851
*/
4952
Controller.prototype.register = function(debuggee, callback) {
50-
var that = this;
51-
5253
var options = {
5354
uri: API + '/debuggees/register',
5455
method: 'POST',
5556
json: true,
5657
body: {debuggee: debuggee}
5758
};
58-
this.debug_.request(options, function(err, body, response) {
59+
this.request(options, function(err, body, response) {
5960
if (err) {
6061
callback(err);
6162
} else if (response.statusCode !== 200) {
@@ -66,7 +67,7 @@ Controller.prototype.register = function(debuggee, callback) {
6667
} else if (body.debuggee.isDisabled) {
6768
callback('Debuggee is disabled on server');
6869
} else {
69-
that.debuggeeId_ = body.debuggee.id;
70+
debuggee.id = body.debuggee.id;
7071
callback(null, body);
7172
}
7273
});
@@ -78,17 +79,17 @@ Controller.prototype.register = function(debuggee, callback) {
7879
* @param {!function(?Error,Object=,Object=)} callback accepting (err, response,
7980
* body)
8081
*/
81-
Controller.prototype.listBreakpoints = function(callback) {
82+
Controller.prototype.listBreakpoints = function(debuggee, callback) {
8283
var that = this;
83-
assert(that.debuggeeId_, 'should register first');
84+
assert(debuggee.id, 'should have a registered debuggee');
8485
var query = {success_on_timeout: true};
8586
if (that.nextWaitToken_) {
86-
query.waitToken = that.nextWaitToken;
87+
query.waitToken = that.nextWaitToken_;
8788
}
8889

89-
var uri = API + '/debuggees/' + encodeURIComponent(that.debuggeeId_) +
90+
var uri = API + '/debuggees/' + encodeURIComponent(debuggee.id) +
9091
'/breakpoints?' + qs.stringify(query);
91-
that.debug_.request({uri: uri, json: true}, function(err, body, response) {
92+
that.request({uri: uri, json: true}, function(err, body, response) {
9293
if (!response) {
9394
callback(err || new Error('unknown error - request response missing'));
9495
return;
@@ -114,25 +115,25 @@ Controller.prototype.listBreakpoints = function(callback) {
114115
* @param {!Breakpoint} breakpoint
115116
* @param {!Function} callback accepting (err, body)
116117
*/
117-
Controller.prototype.updateBreakpoint = function(breakpoint, callback) {
118-
assert(this.debuggeeId_, 'should register first');
118+
Controller.prototype.updateBreakpoint = function(breakpoint, debuggee, callback) {
119+
assert(debuggee.id, 'should have a registered debuggee');
119120

120121
breakpoint.action = 'capture';
121122
breakpoint.isFinalState = true;
122123
var options = {
123-
uri: API + '/debuggees/' + encodeURIComponent(this.debuggeeId_) +
124+
uri: API + '/debuggees/' + encodeURIComponent(debuggee.id) +
124125
'/breakpoints/' + encodeURIComponent(breakpoint.id),
125126
json: true,
126127
method: 'PUT',
127-
body: {debuggeeId: this.debuggeeId_, breakpoint: breakpoint}
128+
body: {debuggeeId: debuggee.id, breakpoint: breakpoint}
128129
};
129130

130131
// We need to have a try/catch here because a JSON.stringify will be done
131132
// by request. Some V8 debug mirror objects get a throw when we attempt to
132133
// stringify them. The try-catch keeps it resilient and avoids crashing the
133134
// user's app.
134135
try {
135-
this.debug_.request(options,
136+
this.request(options,
136137
function(err, body, response) { callback(err, body); });
137138
} catch (error) {
138139
callback(error);

‎src/debuggee.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ var _ = require('lodash');
4242
* @param {?boolean} onGCP - set to true when the debuggee is running inside
4343
* Google Cloud Platform.
4444
*/
45-
function Debuggee(projectId, uid, serviceContext, sourceContext, description,
46-
errorMessage, onGCP) {
45+
function Debuggee(projectId, uid, serviceContext, sourceContext,
46+
description, errorMessage, onGCP) {
4747
if (!(this instanceof Debuggee)) {
4848
return new Debuggee(projectId, uid, serviceContext, sourceContext,
4949
description, errorMessage, onGCP);

‎system-test/test-controller.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe('Controller', function() {
5252
controller.register(debuggee, function(err, body) {
5353
assert.ifError(err, 'should be able to register successfull');
5454

55-
controller.listBreakpoints(function(err, response, body) {
55+
controller.listBreakpoints(debuggee, function(err, response, body) {
5656
assert.ifError(err, 'should successfully list breakpoints');
5757
assert.ok(body);
5858
assert.ok(body.nextWaitToken);

‎test/test-controller.js‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,7 @@ var api = '/v2/controller';
4545

4646
nock.disableNetConnect();
4747

48-
describe('Debuglet API', function() {
49-
50-
var controller = new Controller(fakeDebug);
51-
52-
it('should return an instance when constructed', function() {
53-
assert.ok(controller);
54-
});
48+
describe('Controller API', function() {
5549

5650
describe('register', function() {
5751
it('should get a debuggeeId', function(done) {
@@ -61,10 +55,10 @@ describe('Debuglet API', function() {
6155
.reply(200,
6256
{debuggee: {id: 'fake-debuggee'}, activePeriodSec: 600});
6357
var debuggee = new Debuggee('fake-project', 'fake-id');
58+
var controller = new Controller(fakeDebug);
6459
controller.register(debuggee, function(err, result) {
6560
assert(!err, 'not expecting an error');
6661
assert.equal(result.debuggee.id, 'fake-debuggee');
67-
assert.equal(controller.debuggeeId_, 'fake-debuggee');
6862
scope.done();
6963
done();
7064
});
@@ -86,7 +80,6 @@ describe('Debuglet API', function() {
8680
controller.register(debuggee, function(err, result) {
8781
assert(!err, 'not expecting an error');
8882
assert.equal(result.debuggee.id, 'fake-debuggee');
89-
assert.equal(controller.debuggeeId_, 'fake-debuggee');
9083
scope.done();
9184
delete process.env.GCLOUD_PROJECT;
9285
utils.getProjectNumber = oldProjNum;
@@ -105,6 +98,7 @@ describe('Debuglet API', function() {
10598
activePeriodSec: 600,
10699
});
107100
var debuggee = new Debuggee('fake-project', 'fake-id');
101+
var controller = new Controller(fakeDebug);
108102
controller.register(debuggee, function(err/*, result*/) {
109103
assert(err, 'expected an error');
110104
scope.done();
@@ -125,6 +119,7 @@ describe('Debuglet API', function() {
125119
activePeriodSec: 600
126120
});
127121
var debuggee = new Debuggee('fake-project', 'fake-id');
122+
var controller = new Controller(fakeDebug);
128123
controller.register(debuggee, function(err/*, result*/) {
129124
assert.ifError(err);
130125
done();
@@ -136,7 +131,9 @@ describe('Debuglet API', function() {
136131
.get(api + '/debuggees/fake-debuggee/breakpoints?success_on_timeout=true')
137132
.reply(200, { kind: 'whatever' });
138133

139-
controller.listBreakpoints(function(err, response, result) {
134+
var debuggee = { id: 'fake-debuggee' };
135+
var controller = new Controller(fakeDebug);
136+
controller.listBreakpoints(debuggee, function(err, response, result) {
140137
assert(!err, 'not expecting an error');
141138
assert(!result.breakpoints, 'should not have a breakpoints property');
142139
scope.done();
@@ -151,7 +148,9 @@ describe('Debuglet API', function() {
151148
var scope = nock(url)
152149
.get(api + '/debuggees/fake-debuggee/breakpoints?success_on_timeout=true')
153150
.reply(200, invalidResponse);
154-
controller.listBreakpoints(function(err, response, result) {
151+
var debuggee = { id: 'fake-debuggee' };
152+
var controller = new Controller(fakeDebug);
153+
controller.listBreakpoints(debuggee, function(err, response, result) {
155154
assert(!err, 'not expecting an error');
156155
assert(!result.breakpoints, 'should not have breakpoints property');
157156
scope.done();
@@ -165,7 +164,9 @@ describe('Debuglet API', function() {
165164
var scope = nock(url)
166165
.get(api + '/debuggees/fake-debuggee/breakpoints?success_on_timeout=true')
167166
.reply(403);
168-
controller.listBreakpoints(function(err, response, result) {
167+
var debuggee = { id: 'fake-debuggee' };
168+
var controller = new Controller(fakeDebug);
169+
controller.listBreakpoints(debuggee, function(err, response, result) {
169170
assert(err instanceof Error, 'expecting an error');
170171
assert(!result, 'should not have a result');
171172
scope.done();
@@ -179,8 +180,9 @@ describe('Debuglet API', function() {
179180
.reply(200, {
180181
wait_expired: true
181182
});
182-
183-
controller.listBreakpoints(function(err, response, result) {
183+
var debuggee = { id: 'fake-debuggee' };
184+
var controller = new Controller(fakeDebug);
185+
controller.listBreakpoints(debuggee, function(err, response, result) {
184186
assert.ifError(err, 'not expecting an error');
185187
assert(response.body.wait_expired, 'should have expired set');
186188
scope.done();
@@ -200,8 +202,9 @@ describe('Debuglet API', function() {
200202
.reply(200, {
201203
breakpoints: breakpoints
202204
});
203-
204-
controller.listBreakpoints(function(err, response, result) {
205+
var debuggee = { id: 'fake-debuggee' };
206+
var controller = new Controller(fakeDebug);
207+
controller.listBreakpoints(debuggee, function(err, response, result) {
205208
assert(!err, 'not expecting an error');
206209
assert(result.breakpoints, 'should have a breakpoints property');
207210
var bps = result.breakpoints;
@@ -222,7 +225,9 @@ describe('Debuglet API', function() {
222225
breakpoint: breakpoint
223226
})
224227
.reply(200, { kind: 'debugletcontroller#updateActiveBreakpointResponse'});
225-
controller.updateBreakpoint(breakpoint,
228+
var debuggee = { id: 'fake-debuggee' };
229+
var controller = new Controller(fakeDebug);
230+
controller.updateBreakpoint(breakpoint, debuggee,
226231
function(err, result) {
227232
assert(!err, 'not expecting an error');
228233
assert.equal(result.kind, 'debugletcontroller#updateActiveBreakpointResponse');

0 commit comments

Comments
 (0)