jamielesouef / angular-trackjs Goto Github PK

In the exceptionHandlerDecorator

                if ($window.trackJs) {
                    $window.trackJs.track(exception);
                }

                $delegate(exception, cause);

the exception is sent to TrackJs, but then passed to Angular via the $delegate(exception, cause); line. This causes errors to be tracked twice - first by angular-trackjs, and then by the console.error call in Angular's $ExceptionHandlerProvider (console messages get picked up by TrackJS too).

Apart from duplicate logging, the issue with Angular's logging is that it appends the stack to the message, causing super long messages and thus non-working error detail URLs* in the current version of the TrackJS UI, as well as fragmentation of the error into multiple messages because the .toString() 'footprint' of the function that threw often varies in different environments and calling contexts.

Suggested fix

1. simple version: add an else:

                if ($window.trackJs) {
                    $window.trackJs.track(exception);
                } else {
                    $delegate(exception, cause);
                } 

or drop the $delegate line altogether,
to eat the error.

2. harder but probably the better:
   Add a config that allows whitelisting or blacklisting of environments, so that e.g. in production the errors are not delegated to Angular's own handling. (As suggested here http://trackjs.com/blog/todo-error-handling-angular/ or here https://docs.angularjs.org/api/ng/service/$exceptionHandler )

example of a error detail URL from Angular:
https://my.trackjs.com/messages/filtered?message=%5b%24rootScope%3ainfdig%5d+10+%24digest()+iterations+reached.+Aborting!%0aWatchers+fired+in+the+last+5+iterations%3a+%5b%5b%22fn%3a+function+()+%7b+ANON+FN+CODE+%7d%3b+changeCntr+NEW+and+OLD%22%5d%2c%5b%22fn%3a+function+()+%7b+ANON+FN+CODE+%7d%3b+changeCntr+NEW+and+OLD%22%5d%2c%5b%22fn%3a+function+()+%7b+ANON+FN+CODE+%7d%3b+changeCntr+NEW+and+OLD%22%5d%2c%5b%22fn%3a+function+()+%7b+ANON+FN+CODE+%7d%3b+changeCntr+NEW+and+OLD%22%5d%2c%5b%22fn%3a+function+()+%7b+ANON+FN+CODE+%7d%3b+changeCntr+NEW+and+OLD%22%5d%5d%0ahttp%3a%2f%2ferrors.angularjs.org%2f1.2.13%2f%24rootScope%2finfdig%3fp0%3d10%26p1%3d%255B%255B%2522fn%253A%2520function%2520()%257Bvar%2520a%253De.url()%252Cb%253Di.%2524%2524replace%253Breturn%2520n%2526%2526a%253D%253Di.absUrl()%257C%257C(n%252B%252B%252Cd.%2524evalAsync(function()%257Bd.%2524broadcast(%255C%2522%2524locationChangeStart%255C%2522%252Ci.absUrl()%252Ca).defaultPrevented%253Fi.%2524%2524parse(a)%253A(e.url(i.absUrl()%252Cb)%252Ch(a))%257D))%252Ci.%2524%2524replace%253D!1%252Cn%257D%253B%2520newVal%253A%25208%253B%2520oldVal%253A%25207%2522%255D%252C%255B%2522fn%253A%2520function%2520()%257Bvar%2520a%253De.url()%252Cb%253Di.%2524%2524replace%253Breturn%2520n%2526%2526a%253D%253Di.absUrl()%257C%257C(n%252B%252B%252Cd.%2524evalAsync(function()%257Bd.%2524broadcast(%255C%2522%2524locationChangeStart%255C%2522%252Ci.absUrl()%252Ca).defaultPrevented%253Fi.%2524%2524parse(a)%253A(e.url(i.absUrl()%252Cb)%252Ch(a))%257D))%252Ci.%2524%2524replace%253D!1%252Cn%257D%253B%2520newVal%253A%25209%253B%2520oldVal%253A%25208%2522%255D%252C%255B%2522fn%253A%2520function%2520()%257Bvar%2520a%253De.url()%252Cb%253Di.%2524%2524replace%253Breturn%2520n%2526%2526a%253D%253Di.absUrl()%257C%257C(n%252B%252B%252Cd.%2524evalAsync(function()%257Bd.%2524broadcast(%255C%2522%2524locationChangeStart%255C%2522%252Ci.absUrl()%252Ca).defaultPrevented%253Fi.%2524%2524parse(a)%253A(e.url(i.absUrl()%252Cb)%252Ch(a))%257D))%252Ci.%2524%2524replace%253D!1%252Cn%257D%253B%2520newVal%253A%252010%253B%2520oldVal%253A%25209%2522%255D%252C%255B%2522fn%253A%2520function%2520()%257Bvar%2520a%253De.url()%252Cb%253Di.%2524%2524replace%253Breturn%2520n%2526%2526a%253D%253Di.absUrl()%257C%257C(n%252B%252B%252Cd.%2524evalAsync(function()%257Bd.%2524broadcast(%255C%2522%2524locationChangeStart%255C%2522%252Ci.absUrl()%252Ca).defaultPrevented%253Fi.%2524%2524parse(a)%253A(e.url(i.absUrl()%252Cb)%252Ch(a))%257D))%252Ci.%2524%2524replace%253D!1%252Cn%257D%253B%2520newVal%253A%252011%253B%2520oldVal%253A%252010%2522%255D%252C%255B%2522fn%253A%2520function%2520()%257Bvar%2520a%253De.url()%252Cb%253Di.%2524%2524replace%253Breturn%2520n%2526%2526a%253D%253Di.absUrl()%257C%257C(n%252B%252B%252Cd.%2524evalAsync(function()%257Bd.%2524broadcast(%255C%2522%2524locationChangeStart%255C%2522%252Ci.absUrl()%252Ca).defaultPrevented%253Fi.%2524%2524parse(a)%253A(e.url(i.absUrl()%252Cb)%252Ch(a))%257D))%252Ci.%2524%2524replace%253D!1%252Cn%257D%253B%2520newVal%253A%252012%253B%2520oldVal%253A%252011%2522%255D%255D )

Thanks for the updates.

1. In validateError() we currently check against payload.network[0], but e.g. in my case the network array contains more entries and my particular error is at index 1. Can we fix this?

2. The logError variable in onError() actually means "ignore error". Can it be renamed? It's quite confusing.

3. Could we add a criterium to ignore based on the URL of the page where the error occurred (not just based on the API URL)?

I'd like to do something like this:

            {
                statusCode: 401,
                statusText: 'Unauthorized',
                method: 'POST',
                url: 'https://api.seek.com.au/v1/authenticate/jobseeker',
                pageUrl: /.*\/login.*/
            },

And then validateError() would need to look similar to this (I just hacked it on to demonstrate):

                    angular.forEach(errorCriteria, function (errorTest, prop) {
                        if (errorTest instanceof RegExp) {
                            matchedCriteria.push(errorTest.test(prop === 'pageUrl' ? payload.url : networkResponse[prop]));
                        } else {
                            matchedCriteria.push(prop === 'pageUrl' ? errorTest === payload.url : errorTest === networkResponse[prop]);
                        }
                    });