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

Added support to send Console error logs to Loggly #6

Merged
merged 5 commits into from Dec 5, 2014

Conversation

@varshneyjayant
Copy link
Contributor

@varshneyjayant varshneyjayant commented Dec 2, 2014

Added a configurable field to the library to send console errors to Loggly. Default value is true.

@vhalbwachs
Copy link
Contributor

@vhalbwachs vhalbwachs commented Dec 2, 2014

We use 4 spaces for indentation, do you mind converting your tabs to spaces to keep things consistent?

@@ -129,6 +133,23 @@
}
}

//send console error messages to Loggly
window.onerror = function (msg, url, line, col){

This comment has been minimized.

@vhalbwachs

vhalbwachs Dec 3, 2014
Contributor

If someone assigns an event handler to window.onerror before our script loads, this will overwrite it.

To be safe, we should probably do the following:
*Wait until a user explicitly sets setConsoleError = true before overriding window.onerror (so basically move window.onerror assignment into setSendConsoleErrors). If setConsoleError is never specified, it's defaulted to false.

*Update readme to explain new proprty (I can help with that if you want)

<script type="text/javascript" src="/js/loggly.tracker.js" async></script>
<script>
  var _LTracker = _LTracker || [];
  _LTracker.push({
    'logglyKey': '8c518f97-e3e0-4bfb-a8ed-582d084a5289',
    'setConsoleError': true
  });
</script> 

*Also, just to be safe, we could also save a reference to window.onerror before overwriting it, then calling that as well so we reduce the risk of breaking functionality on a clients page in case they don't realize some library depends on window.onerror as well. So for example, you could do something like this:

var _onerror = window.onerror;
window.onerror = function(msg, url, line, col) {
    if (tracker.sendConsoleErrors) {
        tracker.push({
            category: 'BrowserJsException',
            exception: {
                message: msg,
                url: url,
                lineno: line,
                colno: col,
            }
        });
    }
    if (_onerror && typeof _onerror === 'function') {
        _onerror.apply(window, arguments);
    }
};
@vhalbwachs
Copy link
Contributor

@vhalbwachs vhalbwachs commented Dec 5, 2014

Nice Work

vhalbwachs added a commit that referenced this pull request Dec 5, 2014
Added support to send Console error logs to Loggly
@vhalbwachs vhalbwachs merged commit 20db73b into loggly:master Dec 5, 2014
Shwetajain148 pushed a commit to Shwetajain148/loggly-jslogger that referenced this pull request Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.