mperdeck / jsnlog.js

Tiny JavaScript logging library, simple and well documented. Lots of options to filter logging data.
js.jsnlog.com
Other
130 stars 43 forks source link

AjaxAppender error handling #11

Closed HarryBurns closed 9 years ago

HarryBurns commented 10 years ago

Hi Matt!

I'm using your jsnlog.js in my angular app. And it will be great to know when AjaxAppender can't send logs to server and why.

Here is your code:

try {
    // .......

    // Send the json to the server.
    // Note that there is no event handling here. If the send is not
    // successful, nothing can be done about it.

    var xhr = new XMLHttpRequest();
    xhr.open('POST', ajaxUrl);

    xhr.setRequestHeader('Content-Type', 'application/json');
    xhr.setRequestHeader('JSNLog-RequestId', JL.requestId);
    xhr.send(json);
} catch (e) { }

There is no error handling at all! For this problem we can add ajaxErrorHandler function to appender (or appender's config), and call this function every time something goes wrong. Something like:

try{
    //...........
    var xhr = new XMLHttpRequest();
    xhr.open('POST', ajaxUrl, true); // async call is better

    xhr.setRequestHeader('Content-Type', 'application/json');
    xhr.setRequestHeader('JSNLog-RequestId', JL.requestId);

    xhr.onreadystatechange = function() {
        if (xhr.readyState != 4) return; // request is completed
        if (xhr.status != 200) {
            ajaxErrorHandler(xhr.status + ": " + xhr.statusText); // call our error handler
        }
    }

    xhr.send(json);

} catch (e) {
    ajaxErrorHandler("Fatal logger exception", e);
}

I wrote this code from my head and didn't test it. I just show the main idea. Can you implement this feature please?

HarryBurns commented 10 years ago

By the way, var xhr = new XMLHttpRequest(); is not correct for some browsers.. Maybe it will be better to use universal functions(there is a lot of it on internet). Something like:

function getXmlHttp(){
  var xmlhttp;
  try {
    xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
  } catch (e) {
    try {
      xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
    } catch (E) {
      xmlhttp = false;
    }
  }
  if (!xmlhttp && typeof XMLHttpRequest!='undefined') {
    xmlhttp = new XMLHttpRequest();
  }
  return xmlhttp;
}

What do you think?

mperdeck commented 10 years ago

Hi Harry,

Thanks for your feedback.

About detecting errors when trying to send a log message to the server - I considered this, but was left with the question of what to do when an error happens. In a production environment, I guess the only reasonable option is to retry a few times in the hope that a retry might succeed. But the only scenario where this would work I guess is when the server is very slow or has a strange intermittent bug. I'm not sure it is worth the extra complexity to cater for this.

About introducing fall backs in case XMLHttpRequest is not available - XMLHttpRequest has been available since IE7, so fall backs become important for users of IE6. IE6 is used by less than 4% of users world wide on average. Most users seem to be in China (up to 20% market share). https://www.modern.ie/en-us/ie6countdown

The problem with trying to maintain compatibility with old browsers is that it takes time and increases the size of jsnlog.js. You pointed out XMLHttpRequest, but there are probably more IE6 issues in jsnlog.js that would need to be fixed, in order to support IE6.

For these reasons, I decided to only support IE8 and up, which supports XMLHttpRequest.

HarryBurns commented 10 years ago

"What to do when an error happens"? I think, the best answer is "to allow the user to process this error". Fow now, I can't even see(from code, I mean) when this exception was thrown.

In my app(it's interprise app for internal network), when something really goes wrong(angular was crashed, for example) - I'd like to show to the user the error text. And if we can't send it to the server - show a request to send the message text to the system administrator. Something like "There was an fatal error! We can't fix this error right now, but you can help us! Just send the information below to admin@whateverdomain.com". And stacktrace below.

About IE6 capability - I get the point. My bad. Thanks for explanation!

mperdeck commented 10 years ago

Ok, I see what you mean re. dealing with failure to send log messages to the server.

So I can get a better grip on the value of this new feature - in your estimate how often do you get this combination of events:

1) There is a fatal error in the client side code (such as Angular has crashed); And 2) jsnlog.js is unable to send the error to the server due to a server problem; And 3) the user is clued in enough to actually send the error message to the admin; And 4) the problem happens only once, and it happens while the server is down (if it happens with some frequency, than you'll find out even if one log message is lost).

HarryBurns commented 10 years ago

Well. This combination - not so often. But anyway it is necessary to have opportunity to trace situations when sending logs to the server was unsuccessful.

Here is real example. We have an enterprise application, it's really big and complex. Our angularjs UI - just one small screw in big mechanism. One morning our local network had some problems - IronPort(it's an firewall) blocked our API server for some reason for just one segment of network. There was only couple active users in this segment. The html and js files contains on other server - so peoples can get the index.html and js files, but can't make the API calls. Because of some architectural features, in such situation the AngularJS can't adequately function - and falls.

That's situation! There was client-side error - only for couple users and people can't work. We unable to send information about crash to server, but application even don't know about it! Sad situation.

In enterprise world we need to know even if something impossible was happened - and log it. If we can't send logs to a server - we need to inform admin in any way, even through user. I use <a> tag with mailto with predefined recipient, body and header, so user don't need to make email message by himself.

mperdeck commented 9 years ago

It is now possible to detect and handle situations where log messages cannot be sent to the server. I've written a page on this at: http://jsnlog.com/Documentation/GetStartedLogging/HandlingLoggingFailures