[node] Protect against more badly behaved request implementations

2 files changed, 70 insertions, 7 deletions

diff --git a/platform/node/index.js b/platform/node/index.js
index e36a47943d..5944a0a27d 100644
--- a/platform/node/index.js
+++ b/platform/node/index.js
@@ -19,14 +19,29 @@ var Map = function(options) {
 
     return new constructor(Object.assign(options, {
         request: function(req) {
-            request(req, function() {
+            // Protect against `request` implementations that call the callback synchronously,
+            // call it multiple times, or throw exceptions.
+            // http://blog.izs.me/post/59142742143/designing-apis-for-asynchrony
+
+            var responded = false;
+            var callback = function() {
                 var args = arguments;
-                // Protect against `request` implementations that call the callback synchronously.
-                // http://blog.izs.me/post/59142742143/designing-apis-for-asynchrony
-                process.nextTick(function() {
-                    req.respond.apply(req, args);
-                });
-            });
+                if (!responded) {
+                    responded = true;
+                    process.nextTick(function() {
+                        req.respond.apply(req, args);
+                    });
+                } else {
+                    console.warn('request function responded multiple times; it should call the callback only once');
+                }
+            };
+
+            try {
+                request(req, callback);
+            } catch (e) {
+                console.warn('request function threw an exception; it should call the callback with an error instead');
+                callback(e);
+            }
         }
     }));
 };
diff --git a/platform/node/test/js/request.test.js b/platform/node/test/js/request.test.js
index 9715633e39..ec2b084f4e 100644
--- a/platform/node/test/js/request.test.js
+++ b/platform/node/test/js/request.test.js
@@ -66,3 +66,51 @@ var test = require('tape');
         });
     });
 });
+
+test(`render reports an error if the request function throws an exception`, function(t) {
+    var map = new mbgl.Map({
+        request: function() {
+            throw new Error('message');
+        }
+    });
+    map.load(mockfs.style_vector);
+    map.render({ zoom: 16 }, function(err, pixels) {
+        t.assert(err);
+        t.assert(/message/.test(err.message));
+        t.assert(!pixels);
+        t.end();
+    });
+});
+
+test(`render ignores request functions throwing an exception after calling the callback`, function(t) {
+    var map = new mbgl.Map({
+        request: function(req, callback) {
+            var data = mockfs.dataForRequest(req);
+            callback(null, { data: data });
+            throw new Error('message');
+        }
+    });
+    map.load(mockfs.style_vector);
+    map.render({ zoom: 16 }, function(err, pixels) {
+        t.error(err);
+        t.assert(pixels);
+        t.end();
+    });
+});
+
+test(`render ignores request functions calling the callback a second time`, function(t) {
+    var map = new mbgl.Map({
+        request: function(req, callback) {
+            var data = mockfs.dataForRequest(req);
+            callback(null, { data: data });
+            callback(null, { data: data });
+        }
+    });
+    map.load(mockfs.style_vector);
+    map.render({ zoom: 16 }, function(err, pixels) {
+        t.error(err);
+        t.assert(pixels);
+        t.end();
+    });
+});
+