[node] Do no throw on map.render(), unless missing parameters

3 files changed, 142 insertions, 176 deletions

diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index 37a7484080..c02286c8d1 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -380,45 +380,58 @@ void NodeMap::ApplyOptions(mbgl::Map* map, v8::Local<v8::Object> obj) {
  * @param {Array<string>} [options.classes=[]] style classes
  * @param {Function} callback
  * @returns {undefined} calls callback
- * @throws {Error} if stylesheet is not loaded or if map is already rendering
+ * @throws {Error} On missing parameters
  */
 void NodeMap::Render(const Nan::FunctionCallbackInfo<v8::Value>& info) {
-    auto nodeMap = Nan::ObjectWrap::Unwrap<NodeMap>(info.Holder());
-    if (!nodeMap->map) {
-        return Nan::ThrowError(releasedMessage());
-    }
 
     if (info.Length() <= 0 || !info[0]->IsObject()) {
-        return Nan::ThrowTypeError("First argument must be an options object");
+        Nan::ThrowError("First argument must be an options object");
+        return;
     }
 
     if (info.Length() <= 1 || !info[1]->IsFunction()) {
-        return Nan::ThrowTypeError("Second argument must be a callback function");
+        Nan::ThrowError("Second argument must be a callback function");
+        return;
     }
 
-    if (!nodeMap->loaded) {
-        return Nan::ThrowTypeError("Style is not loaded");
-    }
+    auto nodeMap = Nan::ObjectWrap::Unwrap<NodeMap>(info.Holder());
+    auto callback = std::make_unique<Nan::Callback>(info[1].As<v8::Function>());
+
+    auto errorCallback = [&] (const char* msg) {
+        v8::Local<v8::Value> argv[] = { Nan::Error(msg) };
+        callback->Call(1, argv);
+    };
 
     if (nodeMap->callback) {
-        return Nan::ThrowError("Map is currently rendering an image");
+        errorCallback("Map is currently rendering an image");
+        return;
+    }
+
+    if (!nodeMap->map) {
+        errorCallback(releasedMessage());
+        return;
+    }
+
+    if (!nodeMap->loaded) {
+        errorCallback("Style is not loaded");
+        return;
     }
 
     try {
         ApplyOptions(nodeMap->map.get(), Nan::To<v8::Object>(info[0]).ToLocalChecked());
     } catch (std::exception &ex) {
-        return Nan::ThrowError(ex.what());
+        errorCallback(ex.what());
+        return;
     }
 
     try {
         nodeMap->startRender();
     } catch (std::exception &ex) {
-        return Nan::ThrowError(ex.what());
+        errorCallback(ex.what());
+        return;
     }
 
-    assert(!nodeMap->callback);
-    assert(!nodeMap->image.data);
-    nodeMap->callback = std::make_unique<Nan::Callback>(info[1].As<v8::Function>());
+    nodeMap->callback = std::move(callback);
 
     info.GetReturnValue().SetUndefined();
 }
diff --git a/platform/node/test/js/map.test.js b/platform/node/test/js/map.test.js
index 6f39d85a4f..dcc9bf83bb 100644
--- a/platform/node/test/js/map.test.js
+++ b/platform/node/test/js/map.test.js
@@ -428,12 +428,9 @@ test('Map', function(t) {
         t.test('requires a style to be set', function(t) {
             var map = new mbgl.Map(options);
 
-            t.throws(function() {
-                map.render({}, function() {});
-            }, /Style is not loaded/);
+            map.render({}, function(err) { if (err) t.end(); });
 
             map.release();
-            t.end();
         });
 
         t.test('returns an error delayed', function(t) {
@@ -516,17 +513,15 @@ test('Map', function(t) {
             render();
         });
 
-        t.test('throws if called in parallel', function(t) {
+        t.test('error if called in parallel', function(t) {
             var map = new mbgl.Map(options);
             map.load(style);
 
-            t.throws(function() {
-                map.render({}, function() {});
-                map.render({}, function() {});
-            }, /Map is currently rendering an image/);
+            t.plan(1);
 
-            map.release();
-            t.end();
+            // First will pass, second will fail
+            map.render({}, function(err) { if (err) t.fail(); });
+            map.render({}, function(err) { if (err) t.pass(); });
         });
 
         // This can't be tested with a test-suite render test because zoom and center
diff --git a/platform/node/test/js/options.test.js b/platform/node/test/js/options.test.js
index 8869e7438e..bb66b3a923 100644
--- a/platform/node/test/js/options.test.js
+++ b/platform/node/test/js/options.test.js
@@ -15,6 +15,44 @@ var options = {
     ratio: 1,
 };
 
+function makeMapPool(size) {
+    var mapPool = [];
+
+    for (var i = 0; i < size; ++i) {
+        var map = new mbgl.Map(options);
+        map.load(mockfs.style_vector);
+
+        mapPool.push(map);
+    }
+
+    return mapPool;
+};
+
+test('Usage', function(t) {
+    t.test('rendering after exception', function(t) {
+        var map = new mbgl.Map(options);
+        map.load(mockfs.style_vector);
+
+        map.render({ debug: { tileBorders: "abc" } }, function(err, pixels) {
+            if (err) {
+                map.render({}, function(err, pixels) { if (!err) t.end(); });
+            }
+        });
+    });
+
+    t.test('default options', function(t) {
+        var map = new mbgl.Map(options);
+        map.load(mockfs.style_vector);
+
+        map.render({}, function(err, pixels) {
+            var expected = PNG.sync.read(fs.readFileSync(path.join(__dirname, '../fixtures/options/expected.png'))).data;
+            var numPixels = pixelmatch(pixels, expected, undefined, 512, 512, { threshold: 0.13 });
+            t.equal(numPixels, 0);
+            t.end();
+        });
+    });
+});
+
 test('Center', function(t) {
     t.test('sanity', function(t) {
         var map = new mbgl.Map(options);
@@ -26,219 +64,139 @@ test('Center', function(t) {
     });
 
     t.test('invalid options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
-
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
-
-        t.throws(function() { map.render({ center: 0 }, callback); });
-        t.throws(function() { map.render({ center: "abc" }, callback); });
-        t.throws(function() { map.render({ center: {} }, callback); });
-        t.throws(function() { map.render({ center: [0, 0, 0] }, callback); });
-        t.throws(function() { map.render({ center: ["abc", "def"] }, callback); });
-        t.throws(function() { map.render({ center: ["123", "123"] }, callback); });
-        t.throws(function() { map.render({ center: [123, "123"] }, callback); });
-        t.throws(function() { map.render({ center: ["123", 123] }, callback); });
-        t.throws(function() { map.render({ center: [-1000, -1000] }, callback); });
-        t.throws(function() { map.render({ center: [1000, 1000] }, callback); });
-
-        t.end();
+        var mapPool = makeMapPool(10);
+        var callback = function(err, pixels) { if (err) t.pass(); };
+
+        t.plan(10);
+
+        mapPool[0].render({ center: 0 }, callback);;
+        mapPool[1].render({ center: "abc" }, callback);
+        mapPool[2].render({ center: {} }, callback);
+        mapPool[3].render({ center: [0, 0, 0] }, callback);
+        mapPool[4].render({ center: ["abc", "def"] }, callback);
+        mapPool[5].render({ center: ["123", "123"] }, callback);
+        mapPool[6].render({ center: [123, "123"] }, callback);
+        mapPool[7].render({ center: ["123", 123] }, callback);
+        mapPool[8].render({ center: [-1000, -1000] }, callback);
+        mapPool[9].render({ center: [1000, 1000] }, callback);
     });
 })
 
 test('Zoom', function(t) {
     t.test('sanity', function(t) {
-        var map1 = new mbgl.Map(options);
-        var map2 = new mbgl.Map(options);
-        var map3 = new mbgl.Map(options);
-
-        map1.load(mockfs.style_vector);
-        map2.load(mockfs.style_vector);
-        map3.load(mockfs.style_vector);
+        var mapPool = makeMapPool(3);
 
         t.plan(3);
 
-        // Should clamp and not throw
-        map1.render({ zoom: 10 }, function() { t.pass(); });
-        map2.render({ zoom: -10 }, function() { t.pass(); });
-        map3.render({ zoom: 100 }, function() { t.pass(); });
+        mapPool[0].render({ zoom: 10 }, function() { t.pass(); });
+        mapPool[1].render({ zoom: -10 }, function() { t.pass(); });
+        mapPool[2].render({ zoom: 100 }, function() { t.pass(); });
     });
 
     t.test('invalid options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
+        var mapPool = makeMapPool(3);
+        var callback = function(err, pixels) { if (err) t.pass(); };
 
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
-
-        t.throws(function() { map.render({ zoom: "abc" }, callback); });
-        t.throws(function() { map.render({ zoom: {} }, callback); });
-        t.throws(function() { map.render({ zoom: [0, 0, 0] }, callback); });
+        t.plan(3);
 
-        t.end();
+        mapPool[0].render({ zoom: "abc" }, callback);
+        mapPool[1].render({ zoom: {} }, callback);
+        mapPool[2].render({ zoom: [0, 0, 0] }, callback);
     });
 });
 
 test('Bearing', function(t) {
     t.test('sanity', function(t) {
-        var map1 = new mbgl.Map(options);
-        var map2 = new mbgl.Map(options);
-        var map3 = new mbgl.Map(options);
-
-        map1.load(mockfs.style_vector);
-        map2.load(mockfs.style_vector);
-        map3.load(mockfs.style_vector);
+        var mapPool = makeMapPool(3);
 
         t.plan(3);
 
-        // Should wrap and not throw
-        map1.render({ bearing: 90 }, function() { t.pass(); });
-        map2.render({ bearing: -1000 }, function() { t.pass(); });
-        map3.render({ bearing: 1000 }, function() { t.pass(); });
+        mapPool[0].render({ bearing: 90 }, function() { t.pass(); });
+        mapPool[1].render({ bearing: -1000 }, function() { t.pass(); });
+        mapPool[2].render({ bearing: 1000 }, function() { t.pass(); });
     });
 
     t.test('invalid options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
-
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
+        var mapPool = makeMapPool(3);
+        var callback = function(err, pixels) { if (err) t.pass(); };
 
-        t.throws(function() { map.render({ bearing: "abc" }, callback); });
-        t.throws(function() { map.render({ bearing: {} }, callback); });
-        t.throws(function() { map.render({ bearing: [0, 0, 0] }, callback); });
+        t.plan(3);
 
-        t.end();
+        mapPool[0].render({ bearing: "abc" }, callback);
+        mapPool[1].render({ bearing: {} }, callback);
+        mapPool[2].render({ bearing: [0, 0, 0] }, callback);
     });
 });
 
 test('Pitch', function(t) {
     t.test('sanity', function(t) {
-        var map1 = new mbgl.Map(options);
-        var map2 = new mbgl.Map(options);
-        var map3 = new mbgl.Map(options);
-
-        map1.load(mockfs.style_vector);
-        map2.load(mockfs.style_vector);
-        map3.load(mockfs.style_vector);
+        var mapPool = makeMapPool(3);
 
         t.plan(3);
 
-        // Should clamp and not throw
-        map1.render({ pitch: 10 }, function() { t.pass(); });
-        map2.render({ pitch: -1000 }, function() { t.pass(); });
-        map3.render({ pitch: 1000 }, function() { t.pass(); });
+        mapPool[0].render({ pitch: 10 }, function() { t.pass(); });
+        mapPool[1].render({ pitch: -1000 }, function() { t.pass(); });
+        mapPool[2].render({ pitch: 1000 }, function() { t.pass(); });
     });
 
     t.test('invalid options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
-
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
+        var mapPool = makeMapPool(3);
+        var callback = function(err, pixels) { if (err) t.pass(); };
 
-        t.throws(function() { map.render({ pitch: "abc" }, callback); });
-        t.throws(function() { map.render({ pitch: {} }, callback); });
-        t.throws(function() { map.render({ pitch: [0, 0, 0] }, callback); });
+        t.plan(3);
 
-        t.end();
+        mapPool[0].render({ pitch: "abc" }, callback);
+        mapPool[1].render({ pitch: {} }, callback);
+        mapPool[2].render({ pitch: [0, 0, 0] }, callback);
     });
 });
 
 test('Classes', function(t) {
     t.test('sanity', function(t) {
-        var map1 = new mbgl.Map(options);
-        var map2 = new mbgl.Map(options);
-
-        map1.load(mockfs.style_vector);
-        map2.load(mockfs.style_vector);
+        var mapPool = makeMapPool(2);
 
         t.plan(2);
 
-        map1.render({ classes: [] }, function() { t.pass(); });
-        map2.render({ classes: ["abc"] }, function() { t.pass(); });
+        mapPool[0].render({ classes: [] }, function() { t.pass(); });
+        mapPool[1].render({ classes: ["abc"] }, function() { t.pass(); });
     });
 
     t.test('invalid options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
-
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
+        var mapPool = makeMapPool(3);
+        var callback = function(err, pixels) { if (err) t.pass(); };
 
-        t.throws(function() { map.render({ classes: "abc" }, callback); });
-        t.throws(function() { map.render({ classes: ["abc", 0] }, callback); });
-        t.throws(function() { map.render({ classes: [0, 0, 0] }, callback); });
+        t.plan(3);
 
-        t.end();
+        mapPool[0].render({ classes: "abc" }, callback);
+        mapPool[1].render({ classes: ["abc", 0] }, callback);
+        mapPool[2].render({ classes: [0, 0, 0] }, callback);
     });
 });
 
 test('Debug', function(t) {
     t.test('sanity', function(t) {
-        var map1 = new mbgl.Map(options);
-        var map2 = new mbgl.Map(options);
-        var map3 = new mbgl.Map(options);
-        var map4 = new mbgl.Map(options);
-        var map5 = new mbgl.Map(options);
-        var map6 = new mbgl.Map(options);
-
-        map1.load(mockfs.style_vector);
-        map2.load(mockfs.style_vector);
-        map3.load(mockfs.style_vector);
-        map4.load(mockfs.style_vector);
-        map5.load(mockfs.style_vector);
-        map6.load(mockfs.style_vector);
+        var mapPool = makeMapPool(6);
 
         t.plan(6);
 
-        map1.render({ debug: [] }, function() { t.pass(); });
-        map2.render({ debug: { tileBorders: true } }, function() { t.pass(); });
-        map3.render({ debug: { parseStatus: true } }, function() { t.pass(); });
-        map4.render({ debug: { timestamps: true } }, function() { t.pass(); });
-        map5.render({ debug: { collision: true } }, function() { t.pass(); });
-        map6.render({ debug: { overdraw: true } }, function() { t.pass(); });
+        mapPool[0].render({ debug: [] }, function() { t.pass(); });
+        mapPool[1].render({ debug: { tileBorders: true } }, function() { t.pass(); });
+        mapPool[2].render({ debug: { parseStatus: true } }, function() { t.pass(); });
+        mapPool[3].render({ debug: { timestamps: true } }, function() { t.pass(); });
+        mapPool[4].render({ debug: { collision: true } }, function() { t.pass(); });
+        mapPool[5].render({ debug: { overdraw: true } }, function() { t.pass(); });
     });
 
     t.test('invalid options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
+        var mapPool = makeMapPool(5);
+        var callback = function(err, pixels) { if (err) t.pass(); };
 
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
-
-        t.throws(function() { map.render({ debug: { tileBorders: "abc" } }, callback); });
-        t.throws(function() { map.render({ debug: { parseStatus: "abc" } }, callback); });
-        t.throws(function() { map.render({ debug: { timestamps: "abc" } }, callback); });
-        t.throws(function() { map.render({ debug: { collision: "abc" } }, callback); });
-        t.throws(function() { map.render({ debug: { overdraw: "abc" } }, callback); });
-
-        t.end();
-    });
-});
+        t.plan(5);
 
-test('Usage', function(t) {
-    t.test('rendering after exception', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
-
-        // Should never render, it will throw
-        var callback = function(err, pixels) { t.fail(); };
-
-        t.throws(function() { map.render({ debug: { tileBorders: "abc" } }, callback); });
-        map.render({}, function(err, pixels) { t.end(); });
-    });
-
-    t.test('default options', function(t) {
-        var map = new mbgl.Map(options);
-        map.load(mockfs.style_vector);
-
-        map.render({}, function(err, pixels) {
-            var expected = PNG.sync.read(fs.readFileSync(path.join(__dirname, '../fixtures/options/expected.png'))).data;
-            var numPixels = pixelmatch(pixels, expected, undefined, 512, 512, { threshold: 0.13 });
-            t.equal(numPixels, 0);
-            t.end();
-        });
+        mapPool[0].render({ debug: { tileBorders: "abc" } }, callback);
+        mapPool[1].render({ debug: { parseStatus: "abc" } }, callback);
+        mapPool[2].render({ debug: { timestamps: "abc" } }, callback);
+        mapPool[3].render({ debug: { collision: "abc" } }, callback);
+        mapPool[4].render({ debug: { overdraw: "abc" } }, callback);
     });
 });