Bug 1456526 - Add Error Checking to Protocol.js WIP
MozReview-Commit-ID: LXayvFcSd3u
--- a/devtools/server/main.js
+++ b/devtools/server/main.js
@@ -10,17 +10,17 @@
*/
var { Ci, Cc } = require("chrome");
var Services = require("Services");
var { ActorPool, OriginalLocation, RegisteredActorFactory,
ObservedActorFactory } = require("devtools/server/actors/common");
var { LocalDebuggerTransport, ChildDebuggerTransport, WorkerDebuggerTransport } =
require("devtools/shared/transport/transport");
var DevToolsUtils = require("devtools/shared/DevToolsUtils");
-var { dumpn } = DevToolsUtils;
+var { dumpn, formatTypeSignature } = DevToolsUtils;
DevToolsUtils.defineLazyGetter(this, "DebuggerSocket", () => {
let { DebuggerSocket } = require("devtools/shared/security/socket");
return DebuggerSocket;
});
DevToolsUtils.defineLazyGetter(this, "Authentication", () => {
return require("devtools/shared/security/auth");
});
@@ -1758,16 +1758,23 @@ DebuggerServerConnection.prototype = {
ret = actor.requestTypes[packet.type].bind(actor)(packet, this);
} catch (error) {
let prefix = "error occurred while processing '" + packet.type;
this.transport.send(this._unknownError(actor.actorID, prefix, error));
} finally {
this.currentPacket = undefined;
}
} else {
+ dump(
+ `Unrecognized packet with type "${packet.type}" and following request type` +
+ ` signiture = \n${formatTypeSignature(packet)}\n`
+ );
+ dump(`known types for this actor are: \n ${
+ Object.keys(actor.requestTypes).join("\n ")
+ }\n`);
ret = { error: "unrecognizedPacketType",
message: ("Actor " + actor.actorID +
" does not recognize the packet type " +
packet.type) };
}
// There will not be a return value if a bulk reply is sent.
if (ret) {
--- a/devtools/shared/DevToolsUtils.js
+++ b/devtools/shared/DevToolsUtils.js
@@ -376,16 +376,51 @@ exports.dumpn = function(str) {
*/
exports.dumpv = function(msg) {
if (flags.wantVerbose) {
exports.dumpn(msg);
}
};
/**
+ * Formats the types of a request in an easy to read way.
+ *
+ * @param object
+ * The object being traversed.
+ * @param depth
+ * The indentation of this segment of string.
+ */
+function formatTypeSignature(object, depth = 0) {
+ function typeSignatureToString(string, key) {
+ if (key === "to" || key === "type") {
+ return string;
+ }
+ const padding = " ".repeat(depth);
+ const value = object[key];
+ const type = typeof value;
+ let newSegment;
+ if (type === "object") {
+ const stringOutput = formatTypeSignature(value, depth + 1);
+ newSegment = `${padding}${key}: {\n${stringOutput}${padding}}\n`;
+ } else {
+ newSegment = `${padding}${key}: ${type}\n`;
+ }
+ return string + newSegment;
+ }
+
+ if (!object) {
+ return "null";
+ }
+
+ return Object.keys(object).reduce(typeSignatureToString, "");
+}
+
+exports.formatTypeSignature = formatTypeSignature;
+
+/**
* Defines a getter on a specified object that will be created upon first use.
*
* @param object
* The object to define the lazy getter on.
* @param name
* The name of the getter to define on object.
* @param lambda
* A function that returns what the getter should return. This will
--- a/devtools/shared/protocol.js
+++ b/devtools/shared/protocol.js
@@ -4,16 +4,17 @@
"use strict";
const { extend } = require("devtools/shared/extend");
var EventEmitter = require("devtools/shared/event-emitter");
var {getStack, callFunctionWithAsyncStack} = require("devtools/shared/platform/stack");
var {settleAll} = require("devtools/shared/DevToolsUtils");
var {lazyLoadSpec, lazyLoadFront} = require("devtools/shared/specs/index");
+var { formatTypeSignature } = require("devtools/shared/DevToolsUtils");
// Bug 1454373: devtools/shared/defer still uses Promise.jsm which is slower
// than DOM Promises. So implement our own copy of `defer` based on DOM Promises.
function defer() {
let resolve, reject;
let promise = new Promise(function() {
resolve = arguments[0];
reject = arguments[1];
@@ -510,17 +511,17 @@ Arg.prototype = {
read: function(v, ctx, outArgs) {
outArgs[this.index] = this.type.read(v, ctx);
},
describe: function() {
return {
_arg: this.index,
- type: this.type.name,
+ type: this.type.name,
};
}
};
// Outside of protocol.js, Arg is called as factory method, without the new keyword.
exports.Arg = function(index, type) {
return new Arg(index, type);
};
@@ -703,27 +704,105 @@ Request.prototype = {
* @returns an arguments array
*/
read: function(packet, ctx) {
let fnArgs = [];
for (let templateArg of this.args) {
let arg = templateArg.placeholder;
let path = templateArg.path;
let name = path[path.length - 1];
+ typeCheck(packet, arg.type.name, name);
arg.read(getPath(packet, path), ctx, fnArgs, name);
}
return fnArgs;
},
describe: function() {
return describeTemplate(this.template);
}
};
/**
+ * Checks only the primitive types on a function, bails if it finds a custom type.
+ * Returns true if typecheck passes, false if it fails and emitts an error message.
+ *
+ * @param object packet
+ * The request packet.
+ * @param string typeName
+ * The name of the type being checked.
+ * @param string name
+ * The name of the type being checked.
+ */
+function typeCheck(packet, typeName, argName) {
+ const arg = packet[argName];
+ const type = types.getType(typeName);
+
+ if (type.primitive) {
+ if (typeName.includes(":")) {
+ const isArray = Array.isArray(arg);
+ const subtype = typeName.split(":").pop();
+ if (typeName.includes("array") && !isArray) {
+ console.error(new TypeError(
+ `Expected an array for the argument "${argName}" in packet of ` +
+ `type ${packet.type}\n`
+ ));
+ return false;
+ }
+ if (isArray && typeof arg[0] !== subtype) {
+ console.error(new TypeError(
+ `Expected an array populated with subtype "${subtype}" for ${argName} in ` +
+ `packet ${packet.type}. Instead found an array of ${typeof arg[0]}\n`
+ ));
+ return false;
+ }
+ if (typeName.includes("nullable") && !arg) {
+ return true;
+ }
+ }
+ if (typeof arg !== typeName) {
+ console.error(new TypeError(
+ `Expected the type ${typeName} as the argument for ${packet.type}, received ` +
+ `an argument with ${typeof arg} instead\n`
+ ));
+ return false;
+ }
+ return true;
+ }
+
+ return typeCheckDict(packet, typeName, arg);
+}
+
+function typeCheckDict(packet, typeName) {
+ const { from, to, ...arg } = packet;
+ if (typeof arg !== "object") {
+ console.error(new TypeError(
+ `Expected a DictType "${typeName}" as the argument for ${packet.type}, received ` +
+ `an argument with ${typeof arg} instead\n`
+ ));
+ return false;
+ }
+ const type = types.getType(typeName);
+
+ if (!type.specializations) {
+ // we have no way of knowing more about this type at this point.
+ return true;
+ }
+
+ for (const key in Object.values(type.specializations)) {
+ dump(`%%%%%%%%%%%%%%%%%%%% ${key}\n`);
+ if (arg[key]) {
+ dump(`%%%%%%%%%%%%%%%%%%%% hi\n`);
+
+ } else {
+ }
+ }
+}
+
+
+/**
* Manages a response template.
*
* @param object template
* The response template.
* @construcor
*/
var Response = function(template = {}) {
this.template = template;
@@ -1081,16 +1160,23 @@ var generateActorSpec = function(actorDe
actorSpec.methods.push(spec);
}
}
// Find additional method specifications
if (actorDesc.methods) {
for (let name in actorDesc.methods) {
let methodSpec = actorDesc.methods[name];
+ if (methodSpec.request && methodSpec.request.type) {
+ console.warn(
+ `Request object for protocol method "${name}" has a "type" field and this ` +
+ "will override the spec name. Have you use an Option or Arg without a " +
+ "wrapping it in request object?\n"
+ );
+ }
let spec = {};
spec.name = methodSpec.name || name;
spec.request = new Request(Object.assign({type: spec.name},
methodSpec.request || undefined));
spec.response = new Response(methodSpec.response || undefined);
spec.release = methodSpec.release;
spec.oneway = methodSpec.oneway;
@@ -1119,17 +1205,17 @@ var generateActorSpec = function(actorDe
exports.generateActorSpec = generateActorSpec;
/**
* Generates request handlers as described by the given actor specification on
* the given actor prototype. Returns the actor prototype.
*/
var generateRequestHandlers = function(actorSpec, actorProto) {
if (actorProto._actorSpec) {
- throw new Error("actorProto called twice on the same actor prototype!");
+ // throw new Error("actorProto called twice on the same actor prototype!");
}
actorProto.typeName = actorSpec.typeName;
// Generate request handlers for each method definition
actorProto.requestTypes = Object.create(null);
actorSpec.methods.forEach(spec => {
let handler = function(packet, conn) {
@@ -1137,16 +1223,24 @@ var generateRequestHandlers = function(a
let args;
try {
args = spec.request.read(packet, this);
} catch (ex) {
console.error("Error reading request: " + packet.type);
throw ex;
}
+ if (args.length !== this[spec.name].length) {
+ console.error(new TypeError(
+ `Actor type "${actorSpec.typeName}" has a spec which expects ` +
+ `${args.length} arguments to be passed to the "${spec.name}" method, ` +
+ `but "${spec.name}" expects ${this[spec.name].length}`
+ ));
+ }
+
let ret = this[spec.name].apply(this, args);
let sendReturn = (retToSend) => {
if (spec.oneway) {
// No need to send a response.
return;
}
@@ -1207,16 +1301,17 @@ var ActorClassWithSpec = function(actorS
}
// Existing Actors are relying on the initialize instead of constructor methods.
let cls = function() {
let instance = Object.create(cls.prototype);
instance.initialize.apply(instance, arguments);
return instance;
};
+
cls.prototype = extend(Actor.prototype, generateRequestHandlers(actorSpec, actorProto));
return cls;
};
exports.ActorClassWithSpec = ActorClassWithSpec;
/**
* Base class for client-side actor fronts.