Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

debugger: refactor, no more res.success checks #1779

Closed
wants to merge 2 commits into from

2 participants

@indutny
Owner

I'm sick of stupid, repeating checks like res.success || res.message.
Time to make changes - refactored all code, so error handling is working automatically, like in any normal node.js script.

@ry
ry commented

breaks test/simple/test-debugger-client.js

@indutny
Owner

@ryah fixed, thanks

@ry ry closed this in 1b8b097
@sam-github sam-github referenced this pull request from a commit in sam-github/node
@rvagg rvagg 2015-05-24 io.js v2.1.0 Release
PR-URL: nodejs/io.js#1777

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) #1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) #1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) #1699 #1768 #1779.
* npm: Upgraded to v2.10.1, release notes can be found in
  https://github.com/npm/npm/releases/tag/v2.10.1 and
  https://github.com/npm/npm/releases/tag/v2.10.0.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) #1749.
4d8f4d5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 28, 2011
  1. @indutny
  2. @indutny

    debugger: fix client-test

    indutny authored
This page is out of date. Refresh to see the latest.
Showing with 120 additions and 109 deletions.
  1. +112 −103 lib/_debugger.js
  2. +8 −6 test/simple/test-debugger-client.js
View
215 lib/_debugger.js
@@ -238,7 +238,10 @@ Client.prototype._onResponse = function(res) {
if (cb) {
this._reqCallbacks.splice(index, 1);
handled = true;
- cb(res.body);
+
+ var err = res.success === false && (res.message || true) ||
+ res.body.success === false && (res.body.message || true);
+ cb(err, res.body && res.body.body || res.body, res);
}
if (!handled) this.emit('unhandledResponse', res.body);
@@ -253,8 +256,10 @@ Client.prototype.req = function(req, cb) {
Client.prototype.reqVersion = function(cb) {
- this.req({ command: 'version' } , function(res) {
- if (cb) cb(res.body.V8Version, res.running);
+ cb = cb || function () {};
+ this.req({ command: 'version' } , function(err, body, res) {
+ if (err) return cb(err);
+ cb(null, res.body.body.V8Version, res.body.running);
});
};
@@ -271,16 +276,16 @@ Client.prototype.reqLookup = function(refs, cb) {
}
};
- this.req(req, function(res) {
- if (res.success) {
- for (var ref in res.body) {
- if (typeof res.body[ref] == 'object') {
- self._addHandle(res.body[ref]);
- }
+ cb = cb || function () {};
+ this.req(req, function(err, res) {
+ if (err) return cb(err);
+ for (var ref in res) {
+ if (typeof res[ref] == 'object') {
+ self._addHandle(res[ref]);
}
}
- if (cb) cb(res);
+ cb(null, res);
});
};
@@ -290,20 +295,23 @@ Client.prototype.reqScopes = function(cb) {
command: 'scopes',
arguments: {}
};
- this.req(req, function(res) {
- if (!res.success) return cb(Error(res.message) || true);
- var refs = res.body.scopes.map(function(scope) {
+
+ cb = cb || function () {};
+ this.req(req, function(err, res) {
+ if (err) return cb(err);
+ var refs = res.scopes.map(function(scope) {
return scope.object.ref;
});
- self.reqLookup(refs, function(res) {
- if (!res.success) return cb(Error(res.message) || true)
+ self.reqLookup(refs, function(err, res) {
+ if (err) return cb(err);
- var globals = Object.keys(res.body).map(function(key) {
- return res.body[key].properties.map(function(prop) {
+ var globals = Object.keys(res).map(function(key) {
+ return res[key].properties.map(function(prop) {
return prop.name;
});
});
+
cb(null, globals.reverse());
});
});
@@ -320,12 +328,12 @@ Client.prototype.reqEval = function(expression, cb) {
return;
}
+ cb = cb || function () {};
// Otherwise we need to get the current frame to see which scopes it has.
this.reqBacktrace(function(err, bt) {
if (err || !bt.frames) {
// ??
- cb({});
- return;
+ return cb(null, {});
}
var frame = bt.frames[self.currentFrame];
@@ -353,12 +361,10 @@ Client.prototype._reqFramesEval = function(expression, evalFrames, cb) {
var self = this;
var i = evalFrames.shift();
- this.reqFrameEval(expression, i, function(res) {
- if (res.success) {
- if (cb) cb(res);
- } else {
- self._reqFramesEval(expression, evalFrames, cb);
- }
+ cb = cb || function () {};
+ this.reqFrameEval(expression, i, function(err, res) {
+ if (!err) return cb(null, res);
+ self._reqFramesEval(expression, evalFrames, cb);
});
};
@@ -376,11 +382,10 @@ Client.prototype.reqFrameEval = function(expression, frame, cb) {
req.arguments.frame = frame;
}
- this.req(req, function(res) {
- if (res.success) {
- self._addHandle(res.body);
- }
- if (cb) cb(res);
+ cb = cb || function () {};
+ this.req(req, function(err, res) {
+ if (!err) self._addHandle(res);
+ cb(err, res);
});
};
@@ -388,9 +393,7 @@ Client.prototype.reqFrameEval = function(expression, frame, cb) {
// reqBacktrace(cb)
// TODO: from, to, bottom
Client.prototype.reqBacktrace = function(cb) {
- this.req({ command: 'backtrace' } , function(res) {
- if (cb) cb(!res.success && (res.message || true), res.body);
- });
+ this.req({ command: 'backtrace' } , cb);
};
@@ -412,26 +415,26 @@ Client.prototype.reqBacktrace = function(cb) {
//
Client.prototype.reqScripts = function(cb) {
var self = this;
- this.req({ command: 'scripts' } , function(res) {
- for (var i = 0; i < res.body.length; i++) {
- self._addHandle(res.body[i]);
+ cb = cb || function () {};
+
+ this.req({ command: 'scripts' } , function(err, res) {
+ if (err) return cb(err);
+
+ for (var i = 0; i < res.length; i++) {
+ self._addHandle(res[i]);
}
- if (cb) cb();
+ cb(null);
});
};
Client.prototype.reqContinue = function(cb) {
this.currentFrame = NO_FRAME;
- this.req({ command: 'continue' }, function(res) {
- if (cb) cb(res);
- });
+ this.req({ command: 'continue' }, cb);
};
Client.prototype.listbreakpoints = function(cb) {
- this.req({ command: 'listbreakpoints' }, function(res) {
- if (cb) cb(res);
- });
+ this.req({ command: 'listbreakpoints' }, cb);
};
Client.prototype.setBreakpoint = function(req, cb) {
@@ -440,9 +443,7 @@ Client.prototype.setBreakpoint = function(req, cb) {
arguments: req
};
- this.req(req, function(res) {
- if (cb) cb(res);
- });
+ this.req(req, cb);
};
Client.prototype.clearBreakpoint = function(req, cb) {
@@ -451,9 +452,7 @@ Client.prototype.clearBreakpoint = function(req, cb) {
arguments: req
};
- this.req(req, function(res) {
- if (cb) cb(res);
- });
+ this.req(req, cb);
};
Client.prototype.reqSource = function(from, to, cb) {
@@ -463,9 +462,7 @@ Client.prototype.reqSource = function(from, to, cb) {
toLine: to
};
- this.req(req, function(res) {
- if (cb) cb(!res.success && (res.message || true), res.body);
- });
+ this.req(req, cb);
};
@@ -477,9 +474,7 @@ Client.prototype.step = function(action, count, cb) {
};
this.currentFrame = NO_FRAME;
- this.req(req, function(res) {
- if (cb) cb(res);
- });
+ this.req(req, cb);
};
@@ -507,10 +502,11 @@ Client.prototype.mirrorObject = function(handle, depth, cb) {
return p.ref;
});
- this.reqLookup(propertyRefs, function(res) {
- if (!res.success) {
+ cb = cb || function () {};
+ this.reqLookup(propertyRefs, function(err, res) {
+ if (err) {
console.error('problem with reqLookup');
- if (cb) cb(handle);
+ cb(null, handle);
return;
}
@@ -526,7 +522,7 @@ Client.prototype.mirrorObject = function(handle, depth, cb) {
var keyValues = [];
handle.properties.forEach(function(prop, i) {
- var value = res.body[prop.ref];
+ var value = res[prop.ref];
var mirrorValue;
if (value) {
mirrorValue = value.value ? value.value : value.text;
@@ -547,8 +543,8 @@ Client.prototype.mirrorObject = function(handle, depth, cb) {
};
if (value && value.handle && depth > 0) {
waiting++;
- self.mirrorObject(value, depth - 1, function(result) {
- keyValues[i].value = result;
+ self.mirrorObject(value, depth - 1, function(err, result) {
+ if (!err) keyValues[i].value = result;
waitForOthers();
});
}
@@ -560,7 +556,7 @@ Client.prototype.mirrorObject = function(handle, depth, cb) {
keyValues.forEach(function(kv) {
mirror[kv.name] = kv.value;
});
- cb(mirror);
+ cb(null, mirror);
}
};
});
@@ -577,7 +573,7 @@ Client.prototype.mirrorObject = function(handle, depth, cb) {
val = handle;
}
process.nextTick(function() {
- cb(val);
+ cb(null, val);
});
};
@@ -585,6 +581,7 @@ Client.prototype.mirrorObject = function(handle, depth, cb) {
Client.prototype.fullTrace = function(cb) {
var self = this;
+ cb = cb || function () {};
this.reqBacktrace(function(err, trace) {
if (err) return cb(err);
if (trace.totalFrames <= 0) return cb(Error('No frames'));
@@ -615,17 +612,17 @@ Client.prototype.fullTrace = function(cb) {
refs.push(frame.receiver.ref);
}
- self.reqLookup(refs, function(res) {
- if (!res.success) return cb(res.message || true);
+ self.reqLookup(refs, function(err, res) {
+ if (err) return cb(err);
for (var i = 0; i < trace.frames.length; i++) {
var frame = trace.frames[i];
- frame.script = res.body[frame.script.ref];
- frame.func = res.body[frame.func.ref];
- frame.receiver = res.body[frame.receiver.ref];
+ frame.script = res[frame.script.ref];
+ frame.func = res[frame.func.ref];
+ frame.receiver = res[frame.receiver.ref];
}
- if (cb) cb(null, trace);
+ cb(null, trace);
});
});
};
@@ -873,7 +870,7 @@ Interface.prototype.handleBreak = function(r) {
this.client.currentSourceLineText = r.sourceLineText;
this.client.currentSourceColumn = r.sourceColumn;
this.client.currentFrame = 0;
- this.client.currentScript = r.script.name;
+ this.client.currentScript = r.script && r.script.name;
// Print break data
this.print(SourceInfo(r));
@@ -941,19 +938,15 @@ Interface.prototype.debugEval = function(code, context, filename, callback) {
self.pause();
// Request remote evaluation globally or in current frame
- client.reqFrameEval(code, frame, function(res) {
- if (!res.success) {
- if (res.message) {
- callback(res.message);
- } else {
- callback(null);
- }
+ client.reqFrameEval(code, frame, function(err, res) {
+ if (err) {
+ callback(err);
self.resume(true);
return;
}
// Request object by handles (and it's sub-properties)
- client.mirrorObject(res.body, 3, function(mirror) {
+ client.mirrorObject(res, 3, function(err, mirror) {
callback(null, mirror);
self.resume(true);
});
@@ -1039,8 +1032,12 @@ Interface.prototype.version = function() {
var self = this;
this.pause();
- this.client.reqVersion(function(v) {
- self.print(v);
+ this.client.reqVersion(function(err, v) {
+ if (err) {
+ self.error(err);
+ } else {
+ self.print(v);
+ }
self.resume();
});
};
@@ -1059,7 +1056,9 @@ Interface.prototype.list = function(delta) {
self.pause();
client.reqSource(from, to, function(err, res) {
if (err || !res) {
- return self.error('You can\'t list source code right now');
+ self.error('You can\'t list source code right now');
+ self.resume();
+ return;
}
var lines = res.source.split('\n');
@@ -1107,12 +1106,17 @@ Interface.prototype.backtrace = function() {
self.pause();
client.fullTrace(function(err, bt) {
- if (err) return self.error('Can\'t request backtrace now');
+ if (err) {
+ self.error('Can\'t request backtrace now');
+ self.resume();
+ }
+
if (bt.totalFrames == 0) {
self.print('(empty stack)');
} else {
var trace = [],
firstFrameNative = bt.frames[0].script.isNative;
+
for (var i = 0; i < bt.frames.length; i++) {
var frame = bt.frames[i];
if (!firstFrameNative && frame.script.isNative) break;
@@ -1129,6 +1133,7 @@ Interface.prototype.backtrace = function() {
self.print(trace.join('\n'));
}
+
self.resume();
});
};
@@ -1169,7 +1174,8 @@ Interface.prototype.cont = function() {
this.pause();
var self = this;
- this.client.reqContinue(function() {
+ this.client.reqContinue(function(err) {
+ if (err) self.error(err);
self.resume();
});
};
@@ -1183,7 +1189,8 @@ Interface.stepGenerator = function(type, count) {
var self = this;
self.pause();
- self.client.step(type, count, function(res) {
+ self.client.step(type, count, function(err, res) {
+ if (err) self.error(err);
self.resume();
});
};
@@ -1253,22 +1260,26 @@ Interface.prototype.setBreakpoint = function(script, line,
}
self.pause();
- self.client.setBreakpoint(req, function(res) {
- if (res.success) {
+ self.client.setBreakpoint(req, function(err, res) {
+ if (err) {
+ if (!silent) {
+ self.error(err);
+ }
+ } else {
if (!silent) {
self.list(5);
}
// Try load scriptId and line from response
if (!scriptId) {
- scriptId = res.body.script_id;
- line = res.body.line;
+ scriptId = res.script_id;
+ line = res.line;
}
// If we finally have one - remember this breakpoint
if (scriptId) {
self.client.breakpoints.push({
- id: res.body.breakpoint,
+ id: res.breakpoint,
scriptId: scriptId,
script: (self.client.scripts[scriptId] || {}).name,
line: line,
@@ -1276,10 +1287,6 @@ Interface.prototype.setBreakpoint = function(script, line,
});
}
- } else {
- if (!silent) {
- self.print(res.message || 'error!');
- }
}
self.resume();
});
@@ -1318,12 +1325,12 @@ Interface.prototype.clearBreakpoint = function(script, line) {
};
self.pause();
- self.client.clearBreakpoint(req, function(res) {
- if (res.success) {
+ self.client.clearBreakpoint(req, function(err, res) {
+ if (err) {
+ self.error(err);
+ } else {
self.client.breakpoints.splice(index, 1);
self.list(5);
- } else {
- self.print(req.message || 'error!');
}
self.resume();
});
@@ -1336,12 +1343,12 @@ Interface.prototype.breakpoints = function() {
this.pause();
var self = this;
- this.client.listbreakpoints(function(res) {
- if (res.success) {
- self.print(res.body);
- self.resume();
+ this.client.listbreakpoints(function(err, res) {
+ if (err) {
+ self.error(err);
} else {
- self.error(res.message || 'Some error happened');
+ self.print(res);
+ self.resume();
}
});
};
@@ -1484,7 +1491,9 @@ Interface.prototype.trySpawn = function(cb) {
// since we did debug-brk, we're hitting a break point immediately
// continue before anything else.
- client.reqContinue(function() {
+ client.reqContinue(function(err) {
+ if (err) self.error(err);
+
if (cb) cb();
// Restore breakpoints
View
14 test/simple/test-debugger-client.js
@@ -100,7 +100,8 @@ function addTest (cb) {
addTest(function (client, done) {
console.error("requesting version");
- client.reqVersion(function (v) {
+ client.reqVersion(function (err, v) {
+ assert.ok(!err);
console.log("version: %s", v);
assert.equal(process.versions.v8, v);
done();
@@ -109,7 +110,8 @@ addTest(function (client, done) {
addTest(function (client, done) {
console.error("requesting scripts");
- client.reqScripts(function () {
+ client.reqScripts(function (err) {
+ assert.ok(!err);
console.error("got %d scripts", Object.keys(client.scripts).length);
var foundMainScript = false;
@@ -127,11 +129,11 @@ addTest(function (client, done) {
addTest(function (client, done) {
console.error("eval 2+2");
- client.reqEval("2+2", function (res) {
- assert.ok(res.success);
+ client.reqEval("2+2", function (err, res) {
console.error(res);
- assert.equal('4', res.body.text);
- assert.equal(4, res.body.value);
+ assert.ok(!err);
+ assert.equal('4', res.text);
+ assert.equal(4, res.value);
done();
});
});
Something went wrong with that request. Please try again.