Improve Exception Reporting

[lynchem]

Currently in Kadira we get lots of exceptions that look like this:
[client] Exception in tempalte helper:

As there is no more detail they all get grouped together which can be painful to wade through.

One simple update we could make in Blaze is to pass the name of the helper function to _wrapCatchingException (in lookup.js) so

return Blaze._wrapCatchingExceptions(f, 'template helper').apply(self, args);

Would be instead something like

return Blaze._wrapCatchingExceptions(f, `template helper ${templateFunc.name}`).apply(self, args);

It would be even better if we passed the actual template name into wrapHelper and then we could report that too meaning a unique grouping by template name and helper in Kadira or whatever other error handling program someone is using.

I'm happy to make the PR if whoever monitors this agrees 😃

[lynchem]

Actually that templateFunc.name was always set to "bind" so instead we decided to update wrapHelper to take a name parameter which is uses instead of "template helper". Then when calling wrapHelper we just pass

`${template.viewName} ${name}`

This works nicely so far.

[jamesgibson14]

Do you have a PR for this or just a fork of Blaze? Can you show your code?

[lynchem]

Hi @jamesgibson14. I just monkeypatched it. It would be easy to make a PR as it's a tiny change. Here's our code:

// If `x` is a function, binds the value of `this` for that function
// to the current data context.
function bindDataContext(x) {
    if (typeof x === "function") {
        return function() {
            let data = Blaze.getData();
            if (data === null) {
                data = {};
            }
            return x.apply(data, arguments);
        };
    }
    return x;
}

function wrapHelper(f, templateFunc, name) {
    if (typeof f !== "function") {
        return f;
    }

    return function() {
        let self = this;
        let args = arguments;

        return Blaze.Template._withTemplateInstanceFunc(templateFunc, function() {
            return Blaze._wrapCatchingExceptions(f, name).apply(self, args);
        });
    };
}

Blaze._wrapCatchingExceptions = function(f, where) {
    if (typeof f !== "function") {
        return f;
    }

    return function() {
        try {
            return f.apply(this, arguments);
        } catch (e) {
            if (ObitEnvironment.isTesting()) {
                throw e;
            } else {
                Blaze._reportException(e, `Exception in ${where}:`);
            }
        }
    };
};

Blaze._getTemplateHelper = function(template, name, tmplInstanceFunc) {
    if (template.__helpers.has(name)) {
        let helper = template.__helpers.get(name);
        if (helper === Blaze._OLDSTYLE_HELPER) {
            throw new Meteor.Error("not-suported", "We removed support for old style templates");
        } else if (helper !== null) {
            return wrapHelper(
                bindDataContext(helper),
                tmplInstanceFunc,
                `${template.viewName} ${name}`
            );
        } else {
            return null;
        }
    }
    return null;
};

Blaze._getGlobalHelper = function(name, templateInstance) {
    if (Blaze._globalHelpers[name] !== null) {
        return wrapHelper(
            bindDataContext(Blaze._globalHelpers[name]),
            templateInstance,
            `global helper ${name}`
        );
    }
    return null;
};

[jamesgibson14]

Thank you, I was looking at this same Blaze code and wondering if I could patch it. I will try your code out. It would be nice to get it in as a PR.

…

On Mon, Mar 1, 2021 at 4:03 AM lynchem ***@***.***> wrote: Hi @jamesgibson14 <https://github.com/jamesgibson14>. I just monkeypatched it. It would be easy to make a PR as it's a tiny change. Here's our code: // If `x` is a function, binds the value of `this` for that function // to the current data context. function bindDataContext(x) { if (typeof x === "function") { return function() { let data = Blaze.getData(); if (data === null) { data = {}; } return x.apply(data, arguments); }; } return x; } function wrapHelper(f, templateFunc, name) { if (typeof f !== "function") { return f; } return function() { let self = this; let args = arguments; return Blaze.Template._withTemplateInstanceFunc(templateFunc, function() { return Blaze._wrapCatchingExceptions(f, name).apply(self, args); }); }; } Blaze._wrapCatchingExceptions = function(f, where) { if (typeof f !== "function") { return f; } return function() { try { return f.apply(this, arguments); } catch (e) { if (ObitEnvironment.isTesting()) { throw e; } else { Blaze._reportException(e, `Exception in ${where}:`); } } }; }; Blaze._getTemplateHelper = function(template, name, tmplInstanceFunc) { if (template.__helpers.has(name)) { let helper = template.__helpers.get(name); if (helper === Blaze._OLDSTYLE_HELPER) { throw new Meteor.Error("not-suported", "We removed support for old style templates"); } else if (helper !== null) { return wrapHelper( bindDataContext(helper), tmplInstanceFunc, `${template.viewName} ${name}` ); } else { return null; } } return null; }; Blaze._getGlobalHelper = function(name, templateInstance) { if (Blaze._globalHelpers[name] !== null) { return wrapHelper( bindDataContext(Blaze._globalHelpers[name]), templateInstance, `global helper ${name}` ); } return null; }; — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#291 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFY6KQQUFWW3OKOGZPUJVDTBNYBRANCNFSM4G4DRQLQ> .

[filipenevola]

I'm closing this issue because it's too old.

If you think this issue is still relevant please open a new one.

Why? We need to pay attention to all the open items so we should keep opened only items where people are involved.

[lynchem]

@filipenevola - I can understand bugs quickly becoming irrelevant on actively developed codebases but this is hardly the case here. Here's the last two releases.

v2.3.4, 2019-Dec-13
jquery 3 support #299
v2.3.2, 2017-Mar-21
Made beautification of compiled spacebars code happen only on the server.

There's also been a small bit of discussion of late too. This issue in particular is a really easy win for any blaze user and saved us a few hours of debugging for sure.

[filipenevola]

I can understand bugs quickly becoming irrelevant on actively developed codebases but this is hardly the case here

This is not the case here, I just closed all the old items without comments/activity.

We can re-open or open new issues for the same problem.

As I saw in your original comment that you are willing to make a PR, please go ahead @lynchem

[jamesgibson14]

I just wanted to post here that I tried out @lynchem's "monkey patch" and it works great.

[jankapunkt]

@StorytellerCZ should we consider this for 3.0 ?

[StorytellerCZ]

Yes, that would be a great addition. PRs welcomed!