blaze icon indicating copy to clipboard operation
blaze copied to clipboard

Improve Exception Reporting

Open lynchem opened this issue 5 years ago • 10 comments

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 :smiley:

lynchem avatar Mar 06 '19 13:03 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.

lynchem avatar Mar 06 '19 16:03 lynchem

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

jamesgibson14 avatar Feb 26 '21 04:02 jamesgibson14

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;
};

lynchem avatar Mar 01 '21 11:03 lynchem

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 [email protected] 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 https://github.com/meteor/blaze/issues/291#issuecomment-787861462, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFY6KQQUFWW3OKOGZPUJVDTBNYBRANCNFSM4G4DRQLQ .

jamesgibson14 avatar Mar 09 '21 17:03 jamesgibson14

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.

filipenevola avatar Mar 18 '21 19:03 filipenevola

@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.

lynchem avatar Mar 21 '21 18:03 lynchem

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

filipenevola avatar Mar 22 '21 14:03 filipenevola

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

jamesgibson14 avatar Jun 08 '21 15:06 jamesgibson14

@StorytellerCZ should we consider this for 3.0 ?

jankapunkt avatar Mar 28 '22 07:03 jankapunkt

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

StorytellerCZ avatar Mar 28 '22 09:03 StorytellerCZ