Repository: mozilla/nunjucks
Pull request #504 (2015-09-01 13:54:03)

Catch correctly error from loaders
When an async loader return an error, Nunjucks is crashing the node process because of this [line](https://github.com/mozilla/nunjucks/blob/master/src/environment.js#L225).

This PR fixes this, and add a test for it.


Patch:
diff --git a/src/environment.js b/src/environment.js
index 83d17e3..2c3e280 100644
--- a/src/environment.js
+++ b/src/environment.js
@@ -179,9 +179,12 @@ var Environment = Obj.extend({
             var syncResult;
             var _this = this;
 
-            var createTemplate = function(info) {
-                if(!info) {
-                    var err = new Error('template not found: ' + name);
+            var createTemplate = function(err, info) {
+                if(!info && !err) {
+                    err = new Error('template not found: ' + name);
+                }
+
+                if (err) {
                     if(cb) {
                         cb(err);
                     }
@@ -207,10 +210,13 @@ var Environment = Obj.extend({
             };
 
             lib.asyncIter(this.loaders, function(loader, i, next, done) {
-                function handle(src) {
-                    if(src) {
+                function handle(err, src) {
+                    if(err) {
+                        done(err);
+                    }
+                    else if(src) {
                         src.loader = loader;
-                        done(src);
+                        done(null, src);
                     }
                     else {
                         next();
@@ -221,13 +227,10 @@ var Environment = Obj.extend({
                 name = that.resolveTemplate(loader, parentName, name);
 
                 if(loader.async) {
-                    loader.getSource(name, function(err, src) {
-                        if(err) { throw err; }
-                        handle(src);
-                    });
+                    loader.getSource(name, handle);
                 }
                 else {
-                    handle(loader.getSource(name));
+                    handle(null, loader.getSource(name));
                 }
             }, createTemplate);
 


Test patch:
diff --git a/tests/loader.js b/tests/loader.js
index fd05174..ea2ac54 100644
--- a/tests/loader.js
+++ b/tests/loader.js
@@ -32,5 +32,29 @@
             var parent = env.getTemplate('fake.html');
             expect(parent.render()).to.be('Hello World');
         });
+
+        it('should catch loader error', function(done) {
+            // From Docs: http://mozilla.github.io/nunjucks/api.html#writing-a-loader
+            // We should be able to create a loader that only exposes getSource
+            function MyLoader() {
+                // configuration
+                this.async = true;
+            }
+
+            MyLoader.prototype.getSource = function(s, cb) {
+                setTimeout(function() {
+                    cb(new Error('test'));
+                }, 1);
+            };
+
+            var env = new Environment(new MyLoader(templatesPath));
+            env.getTemplate('fake.html', function(err, parent) {
+                expect(err).to.be.a(Error);
+                expect(parent).to.be(undefined);
+
+                done();
+            });
+
+        });
     });
 })();