4

I want to know your opinions on a function I made, that wraps every method of an object, adding "try catch" handlers to log JavaScript errors on server side.

I don't want to use window.onerror, because of this security restriction, and my scripts are going to be hosted on a different domain on a CDN.

/*
 * object: Object to be wrapped
 * errorHandler: Instance of ErrorHandler Object
 */
function addLog(object, errorHandler) {
    var name, method;

    for (name in object) {
        method = object[name];
        if (typeof method === "function") {
            object[name] = function(method, name) {
                return function() {
                    try {
                        return method.apply(this, arguments);
                    } catch (ex) {
                        ex.message += "; method: '" + name + "'";
                        errorHandler.addToStack(ex.message);
                        throw ex;
                    }
                };
            }(method, name);
        }
    }
    return object;
}

errorHandler.addToStack is a method on a custom object that asynchronously sends error reports to a server using Ajax messages.

I want to wrap every object instantiated on my application with this function, but I'm not sure if:

  • Is this a bad practice?
  • Does it has performance issues?
  • Are there's a better ways for doing this?

Thanks in advance!

Community
  • 1
  • 1
Gabriel Jürgens
  • 3,055
  • 3
  • 20
  • 19

3 Answers3

1

Unlike stricter codes like Java, Javascript doesn't require try/catch for most objects instanciated. It tends to simply just not work if something goes wrong, and doesn't explode or end on you.

Some parts do however end the function, so a better method would be to surround the code as a whole with a try/catch, so that it fails silently, and use the catch to recall the original code while sending off an error report.

Death
  • 1,999
  • 12
  • 14
  • 1
    I don't need to hide errors failing silently, notice the `throw ex;` after `errorHandler.addToStack(ex.message);`. I only need to send error information to a server side log. Thanks for your answer! – Gabriel Jürgens May 28 '12 at 16:59
1

I think that the best way is to avoid try catch statements by preventing errors to occur adding validations and checks, but if you really need to do it this way, I don't think it will have a great performance issue.

Here I made a jspref test to measure it, and the difference shows only in IE, but it's not very significant.

I don't know if the errorHandler method has performance problems or slows down your code, but if it is async, I guess it won't be a problem.

Charles Frezno
  • 26
  • 2
  • I'm a little confused (and curious): in what sense does your "closure try" in this test involve try / **catch**? AFAICT it doesn't do any such thing... – CodeClown42 May 28 '12 at 20:58
  • Sorry @goldilocks, I forgot to add it! I just fixed that, and it throws similar results. Still no much difference. Only something I don't understand, is why in Chrome it runs faster with the closure and the try catch statement!?!?!?! – Charles Frezno May 28 '12 at 21:12
  • Interesting about chrome and good for the perf test (I was the +1), but why did you use a self-executing function (nb, "closure" is a more general concept) in test 2??? I added a "Revision 2" with a "plain try" in it, and, in fact, the "no try" and "plain try" come out almost identical. Ie, the difference between "closure try" and "no try" would seem to have nothing to do with try/catch ;) http://jsperf.com/try-catch-performance-closure/2 *When you perf test something, just test it, not it + some other unrelated thing.* – CodeClown42 May 28 '12 at 21:37
0

you can still send log to server using a global try/catch because exception objects contains the information.

however error object is not standard among browsers, i think

neu-rah
  • 1,662
  • 19
  • 32