Parser syntax errors should be passed to the callback, not thrown

[brettstimmerman]

Originally submitted as akdubya/dustjs#12:

When the parser encounters a syntax error, it currently throws a SyntaxError exception instead of bubbling it up to the render callback like I'd expect. Catching this exception is proving to be a challenge in an Express app. I'd much rather be able to handle it in my callback.

This will throw a SyntaxError in Node (run it as a file, though; the REPL catches and hides the exception):

var dust    = require('dust'),
    context = dust.makeBase();

dust.renderSource('{<body}no end tag', context, function (err, html) {
    if (err) {
        console.log('I got an error!'); // never gets here
    }
});

[jairodemorais]

@brettstimmerman I think that you are right, the exception should not be hided. I will take a look.

@vybs @iamleppert @jimmyhchan what do u think about this issue??

[jleppert]

I'd expect it to work like that too. Ideally in all dust.render methods we should put a try/catch block in there, catch any errors and return the message/object to the callback. Do you agree?

[vybs]

has anyone double condfirmed this? dust.renderSource('{<body}no end tag', context, function (err, html) { if (err) { console.log('I got an error!'); // never gets here } }); i remember it works as expected with dust.stream and dust.render

Since we use precompiled templates, and there is no chance of sycntax issues at runtime. the only error I have found is the template not found

[sielay]

I think it's still relevant in 2.7.4

If you take a look on the lines: https://github.com/linkedin/dustjs/blob/2.7/lib/dust.js#L171 and https://github.com/linkedin/dustjs/blob/2.7/lib/compiler.js#L34

If you implement your own onLoad and it will return content as a string (won't perform dust.compile on it's own) then you'll get error like

Uncaught SyntaxError: Expected end tag for ... but it was not found. At line : 2, column : 1
      at Object.compiler.compile (node_modules/dustjs-linkedin/lib/compiler.js:34:13)
      at done (node_modules/dustjs-linkedin/lib/dust.js:171:49)

I tried on such example of onLoad

(templateName, options, callback) => {

        if (dust.cache[templateName]) {
            return dust.cache[templateName];
        }

        pathResolver(templateName)
            .then(filePath => fs
                .readFile(filePath, 'utf8', (error, content) => {
                    if (error) {
                        debug('error ' + error);
                        return callback(error);
                    }
                    console.log('content', content);
                    callback(null, content);
                }))
            .catch(error => callback(error));
    };

I easilly fixed that in my code in following way, but this require at least proper documenting

(templateName, options, callback) => {

        if (dust.cache[templateName]) {
            return dust.cache[templateName];
        }

        pathResolver(templateName)
            .then(filePath => fs
                .readFile(filePath, 'utf8', (error, content) => {
                    if (error) {
                        debug('error ' + error);
                        return callback(error);
                    }
                    try {
                        callback(null, dust.loadSource(dust.compile(content, templateName)));
                    } catch(err) {
                        callback(err);
                    }

                }))
            .catch(error => callback(error));
    };