Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors in included templates are thrown asynchronously #678

Closed
nicolasartman opened this issue Feb 17, 2016 · 6 comments · Fixed by #1181 · May be fixed by dubscratch/MagicMirror#16
Closed

Errors in included templates are thrown asynchronously #678

nicolasartman opened this issue Feb 17, 2016 · 6 comments · Fixed by #1181 · May be fixed by dubscratch/MagicMirror#16

Comments

@nicolasartman
Copy link

As far as I can tell, this applies to any block error and possibly any error at all

I've attached a folder (nunjucks-repro.zip) with the reproduction for convenience, and here's the code in each file:

repro.js

var nunjucks = require('nunjucks');

try {
    console.log('compiling');
    var template = nunjucks.compile('{% include "included.html" %}', new nunjucks.Environment([
        new nunjucks.FileSystemLoader('./')
    ]));
    console.log('rendering');
    template.render();
    console.log('done rendering');
} catch (err) {
    console.log('caught nunjucks error', err.message);
}

included.html

{% invalidTag %}

I wasn't able to figure out exactly why this is happening, but it appears the flow goes into an asyncIter (despite nothing async being used anywhere) here, and then when it calls render again, it has passed a callback, causing this path to be taken, which erroneously reports the error asynchronously.

I hacked it in my fork by just throwing instead of ever calling callbackAsap, but that's clearly not the right way to fix it.

thanks!

@carljm
Copy link
Contributor

carljm commented Feb 17, 2016

Any chance you could rework that example into a failing test in the nunjucks test suite? That'd be the next step towards fixing this.

@nicolasartman
Copy link
Author

Done! In trying to make a failing test, I also discovered that it doesn't happen with invalid macros, but I'm not sure why. I thought that calling Template.render with a callback was the issue, but I see that's done anytime there's an import or include, and in most cases it works fine (even when there's an error such as the invalid macro) because compile doesn't throw during one of the calls where there's a callback, so it doesn't get reported asynchronously.

You can see the test here—let me know if you want me to issue a PR or anything else.

@carljm
Copy link
Contributor

carljm commented Feb 18, 2016

Yeah, go ahead and make a PR with that failing test, and mention this ticket number in its description. That makes it more visible / easier to find, the next time I get a chance to spend some time working on nunjucks. (Obviously if you're able to figure out a fix in the meantime and add it to the PR, even better!)

Thanks!

@oleics
Copy link

oleics commented Jun 15, 2016

Half an hour of debugging an express-app brought up two lines:

https://github.com/mozilla/nunjucks/blob/1d94ee6/src/environment.js#L454
https://github.com/mozilla/nunjucks/blob/1d94ee6/src/environment.js#L489

Hope this helps.

@joemaller
Copy link

Just ran into this on v3. A typo in tag inside an included file causes nunjucks to throw an async error that can't be caught in a normal try/catch block.

A workaround seems to be calling render or renderString asynchronously. Be careful though, nunjucks may throw multiple errors (possibly once for the erroring include and the parent?), so double-check that async callbacks are only triggered once. (my streams were failing with no writecb in transform class errors before realizing nunjucks was throwing multiple times.)

@rusekr
Copy link

rusekr commented Oct 11, 2017

Catched this error when using express and nunjucks. If included template has error nunjucks render method fires two times: 1. parent template 2. errored included template. And after 2 express rejects with "Error: Can't set headers after they are sent."

Workaround for express to not reject:

    var tplEnv = new nunjucks.Environment(new profileLoader(), {
      autoescape: true
    });

    // by STM ловим все ошибки шаблонов сами и сразу
    ['render', 'renderString'].forEach(function (method) {
      var _original = tplEnv[method];
      tplEnv[method] = function (view, data, cb) {
        var ocb = typeof cb == 'function' ? cb : function () {};
        var _cb = function (err, str) {
          var errMsg = false;
          if (err) {
            errMsg = 'Render error: ' + (err instanceof Object && err.message || JSON.stringify(err));
            debug(errMsg);
          } else {
            return ocb(null, errMsg || str);
          }
        };
        return _original.call(tplEnv, view, data, _cb);
      };
    });
    tplEnv.express(app);

But need a better solution for at least render error on target page, not only in console.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants