First attempt at better error handling, also some general cleanup

src/datatypes/structures.js

@@ -1,4 +1,4 @@
-var getField = require("../utils").getField;
+var { getField, tryCatch, addErrorField } = require("../utils");
 var debug = require("../debug");
 
 module.exports = {
@@ -25,14 +25,26 @@ function readArray(buffer, offset, typeArgs, rootNode) {
   else if (typeof typeArgs.count !== "undefined")
     count = getField(typeArgs.count, rootNode);
   else if (typeof typeArgs.countType !== "undefined") {
-    var countResults = this.read(buffer, offset, { type: typeArgs.countType, typeArgs: typeArgs.countTypeArgs }, rootNode);
+    var countResults;
+    tryCatch(() => {
+      countResults = this.read(buffer, offset, { type: typeArgs.countType, typeArgs: typeArgs.countTypeArgs }, rootNode);
+    }, (e) => {
+      addErrorField(e, "$count");
+      throw e;
+    });
     results.size += countResults.size;
     offset += countResults.size;
     count = countResults.value;
   } else // TODO : broken schema, should probably error out.
     count = 0;
   for(var i = 0; i < count; i++) {
-    var readResults = this.read(buffer, offset, typeArgs.type, rootNode);
+    var readResults;
+    tryCatch(() => {
+      readResults = this.read(buffer, offset, typeArgs.type, rootNode);
+    }, (e) => {
+      addErrorField(e, i);
+      throw e;
+    });
     results.size += readResults.size;
     offset += readResults.size;
     results.value.push(readResults.value);
@@ -43,11 +55,21 @@ function readArray(buffer, offset, typeArgs, rootNode) {
 function writeArray(value, buffer, offset, typeArgs, rootNode) {
   if (typeof typeArgs.count === "undefined" &&
       typeof typeArgs.countType !== "undefined") {
-    offset = this.write(value.length, buffer, offset, { type: typeArgs.countType, typeArgs: typeArgs.countTypeArgs }, rootNode);
+    tryCatch(() => {
+      offset = this.write(value.length, buffer, offset, { type: typeArgs.countType, typeArgs: typeArgs.countTypeArgs }, rootNode);
+    }, (e) => {
+      addErrorField(e, "$count");
+      throw e;
+    });
   } else if (typeof typeArgs.count === "undefined") { // Broken schema, should probably error out
   }
   for(var index in value) {
-    offset = this.write(value[index], buffer, offset, typeArgs.type, rootNode);
+    tryCatch(() => {
+      offset = this.write(value[index], buffer, offset, typeArgs.type, rootNode);
+    }, (e) => {
+      addErrorField(e, i);
+      throw e;
+    });
   }
   return offset;
 }
@@ -56,10 +78,20 @@ function sizeOfArray(value, typeArgs, rootNode) {
   var size = 0;
   if (typeof typeArgs.count === "undefined" &&
       typeof typeArgs.countType !== "undefined") {
-    size = this.sizeOf(value.length, { type: typeArgs.countType, typeArgs: typeArgs.countTypeArgs }, rootNode);
+    tryCatch(() => {
+      size = this.sizeOf(value.length, { type: typeArgs.countType, typeArgs: typeArgs.countTypeArgs }, rootNode);
+    }, (e) => {
+      addErrorField(e, "$count");
+      throw e;
+    });
   }
   for(var index in value) {
-    size += this.sizeOf(value[index], typeArgs.type, rootNode);
+    tryCatch(() => {
+      size += this.sizeOf(value[index], typeArgs.type, rootNode);
+    }, (e) => {
+      addErrorField(e, i);
+      throw e;
+    });
   }
   return size;
 }
@@ -70,16 +102,16 @@ function readContainer(buffer, offset, typeArgs, rootNode) {
     value: {},
     size: 0
   };
-  // BLEIGH. Huge hack because I have no way of knowing my current name.
-  // TODO : either pass fieldInfo instead of typeArgs as argument (bleigh), or send name as argument (verybleigh).
-  // TODO : what I do inside of roblabla/Protocols is have each "frame" create a new empty slate with just a "super" object pointing to the parent.
   var backupThis = rootNode.this;
   rootNode.this = results.value;
   for(var index in typeArgs) {
-    var readResults = this.read(buffer, offset, typeArgs[index].type, rootNode);
+    var readResults;
-    if(readResults == null || readResults.value == null) {
+    tryCatch(() => {
-      continue;
+      readResults = this.read(buffer, offset, typeArgs[index].type, rootNode);
-    }
+    }, (e) => {
+      addErrorField(e, index);
+      throw e;
+    });
     results.size += readResults.size;
     offset += readResults.size;
     results.value[typeArgs[index].name] = readResults.value;
@@ -92,7 +124,12 @@ function writeContainer(value, buffer, offset, typeArgs, rootNode) {
   var backupThis = rootNode.this;
   rootNode.this = value;
   for(var index in typeArgs) {
-    offset = this.write(value[typeArgs[index].name], buffer, offset, typeArgs[index].type, rootNode);
+    tryCatch(() => {
+      offset = this.write(value[typeArgs[index].name], buffer, offset, typeArgs[index].type, rootNode);
+    }, (e) => {
+      addErrorField(e, index);
+      throw e;
+    });
   }
   rootNode.this = backupThis;
   return offset;
@@ -103,7 +140,12 @@ function sizeOfContainer(value, typeArgs, rootNode) {
   var backupThis = rootNode.this;
   rootNode.this = value;
   for(var index in typeArgs) {
-    size += this.sizeOf(value[typeArgs[index].name], typeArgs[index].type, rootNode);
+    tryCatch(() => {
+      size += this.sizeOf(value[typeArgs[index].name], typeArgs[index].type, rootNode);
+    }, (e) => {
+      addErrorField(e, index);
+      throw e;
+    });
   }
   rootNode.this = backupThis;
   return size;

src/transforms/serializer.js

@@ -3,7 +3,7 @@ var protocol = require("../protocol");
 var Transform = require("readable-stream").Transform;
 var debug = require("../debug");
 var assert = require('assert');
-var { getFieldInfo } = require('../utils');
+var { getFieldInfo, tryCatch, addErrorField } = require('../utils');
 
 module.exports.createSerializer = function(obj) {
   return new Serializer(obj);
@@ -72,13 +72,12 @@ function createPacketBuffer(packetId, state, params, isServer) {
   var packet = get(packetId, state, !isServer);
   assert.notEqual(packet, null);
   packet.forEach(function(fieldInfo) {
-    try {
+    tryCatch(() => {
       length += proto.sizeOf(params[fieldInfo.name], fieldInfo.type, params);
-    } catch(e) {
+    }, (e) => {
-      console.log("fieldInfo : " + JSON.stringify(fieldInfo));
+      e.message = "sizeOf error for " + e.field + " : " + e.message;
-      console.log("params : " + JSON.stringify(params));
       throw e;
-    }
+    });
   });
   length += utils.varint[2](packetId);
   var size = length;// + utils.varint[2](length);
@@ -90,7 +89,12 @@ function createPacketBuffer(packetId, state, params, isServer) {
     // TODO : This check belongs to the respective datatype.
     if(typeof value === "undefined" && fieldInfo.type != "count")
       debug(new Error("Missing Property " + fieldInfo.name).stack);
-    offset = proto.write(value, buffer, offset, fieldInfo.type, params);
+    tryCatch(() => {
+      offset = proto.write(value, buffer, offset, fieldInfo.type, params);
+    }, (e) => {
+      e.message = "Write error for " + e.field + " : " + e.message;
+      throw e;
+    });
   });
   return buffer;
 }
@@ -125,11 +129,7 @@ function parsePacketData(buffer, state, isServer, packetsToParse = {"packet": tr
 
   var packetInfo = get(packetId, state, isServer);
   if(packetInfo === null) {
-    return {
+    throw new Error("Unrecognized packetId: " + packetId + " (0x" + packetId.toString(16) + ")")
-      error: new Error("Unrecognized packetId: " + packetId + " (0x" + packetId.toString(16) + ")"),
-      buffer: buffer,
-      results: results
-    };
   } else {
     var packetName = packetNames[state][isServer ? "toServer" : "toClient"][packetId];
     debug("read packetId " + state + "." + packetName + " (0x" + packetId.toString(16) + ")");
@@ -139,31 +139,16 @@ function parsePacketData(buffer, state, isServer, packetsToParse = {"packet": tr
   for(i = 0; i < packetInfo.length; ++i) {
     fieldInfo = packetInfo[i];
     readResults = proto.read(buffer, cursor, fieldInfo.type, results);
-    /* A deserializer cannot return null anymore. Besides, proto.read() returns
+    if(readResults === null)
-     * null when the condition is not fulfilled.
+      throw new Error("A reader returned null. This is _not_ normal");
-     if (!!!readResults) {
+    if(readResults.error)
-     var error = new Error("A deserializer returned null");
+      throw new Error("A reader returned an error using the old method.");
-     error.packetId = packetId;
+    if (readResults.value != null)
-     error.fieldInfo = fieldInfo.name;
+      results[fieldInfo.name] = readResults.value;
-     return {
-     size: length + lengthField.size,
-     error: error,
-     results: results
-     };
-     }*/
-    // TODO : investigate readResults returning null : shouldn't happen.
-    // When there is not enough data to read, we should return an error.
-    // As a general rule, it would be a good idea to introduce a whole bunch
-    // of new error classes to differenciate the errors.
-    if(readResults === null || readResults.value == null) continue;
-    if(readResults.error) {
-      return readResults;
-    }
-    results[fieldInfo.name] = readResults.value;
     cursor += readResults.size;
   }
   if(buffer.length > cursor)
-    debug("Too much data to read for packetId: " + packetId + " (0x" + packetId.toString(16) + ")");
+    throw new Error("Packet data not entirely read");
   debug(results);
   return {
     results: results,

src/utils.js

@@ -1,6 +1,8 @@
 module.exports = {
   getField: getField,
   getFieldInfo: getFieldInfo,
+  addErrorField: addErrorField,
+  tryCatch: tryCatch,
 };
 
 function getField(countField, rootNode) {
@@ -22,3 +24,14 @@ function getFieldInfo(fieldInfo) {
   else
     throw new Error("Not a fieldinfo");
 }
+
+function addErrorField(e, field) {
+  if (e.field)
+    e.field = field + "." + e.field;
+  else
+    e.field = field;
+}
+
+function tryCatch(tryfn, catchfn) {
+  try { tryfn(); } catch (e) { catchfn(e); }
+}

test/test.js

@@ -53,6 +53,12 @@ var defaultServerProps = {
   'motd': 'A Minecraft Server',
 };
 
+function evalCount(count, fields) {
+  if(fields[count["field"]] in count["map"])
+    return count["map"][fields[count["field"]]];
+  return count["default"];
+}
+
 var values = {
   'int': 123456,
   'short': -123,
@@ -63,7 +69,19 @@ var values = {
   'string': "hi hi this is my client string",
   'buffer': new Buffer(8),
   'array': function(typeArgs, packet) {
-    return [getValue(typeArgs.type, packet)];
+    var count;
+    if (typeof typeArgs.count === "object")
+      count = evalCount(typeArgs.count, packet);
+    else if (typeof typeArgs.count !== "undefined")
+      count = getField(typeArgs.count, rootNode);
+    else if (typeof typeArgs.countType !== "undefined")
+      count = 1;
+    var arr = [];
+    while (count > 0) {
+      arr.push(getValue(typeArgs.type, packet));
+      count--;
+    }
+    return arr;
   },
   'container': function(typeArgs, packet) {
     var results = {};