4

I want to extend the general catch prototype of the Promise object, so that I can automatically log the error to out application monitoring whenever a catch block is hit. But I have trouble getting the error object out of the Promise object when trying to extend the catch.

So basically instead of doing this in every then().catch()

axios.get('sample/url')
    .then(response => { stuff })
    .catch(error => {
        newrelic.noticeError(error);
    });

I want to extend the Promise prototype, but fail to get the Error Object out of it.

(function (Promise) {
    const originalCatch = Promise.prototype.catch;

    Promise.prototype.catch = function () {
        console.log('> > > > > > called .catch on %o with arguments: %o', this, arguments);

        if (typeof newrelic !== 'undefined') {
            newrelic.noticeError(arguments[0]);
        } else {
            console.error(arguments);
        }

        return originalCatch.apply(this, arguments);
    };
})(Promise);

Sebsemillia
  • 9,366
  • 2
  • 55
  • 70
  • That was just a copy / paste mistake by me, sorry! I'm still struggling getting the error object out of the `this` or `arguments`, so that I can pass it to `newrelic.noticeError()` though. – Sebsemillia Feb 28 '19 at 11:05
  • Oh, sorry. Misunderstood the question, I'm sorry. You can get the error argument by applying to the original Promise.catch the Promise's `this` and, as argument, you can provide your callback, like this: https://jsfiddle.net/7qgjr6fw/3/ . In that example, `hello` will be passed to `myCaller.noticeError`. That example specifically shows that both callbacks are effectively invoked, both having the same error argument. As a side note, I'm not exactly sure whether extending the `Promise` prototype is a good practice, I would follow other approaches instead. – briosheje Feb 28 '19 at 11:09
  • We did want to prevent that we have to add the line of code for passing the error to the app monitoring in every catch. What other approach would you suggest instead? And thank you! – Sebsemillia Feb 28 '19 at 11:15
  • 1
    I would make a service used in the entire application that holds every single http request and, whenever it catches an error, it fires an event, keeping the promise prototype as it is. However, if you already have the whole application built and making other changes would take too much time, this solution is probably one of the only available ones, unless `axios` provide extra events. Just make sure `noticeError` never fails for some reasons, otherwise debugging or trying to understand why a promise doesn't fire a `catch` can be painful. – briosheje Feb 28 '19 at 11:18
  • 1
    @briosheje [Parenthesis order doesn't matter at all](https://stackoverflow.com/questions/3384504/location-of-parenthesis-for-auto-executing-anonymous-javascript-functions) – Bergi Feb 28 '19 at 11:22
  • @Bergi Oh. Thanks. didn't know that, I've always thought it was relevant. Thanks for pointing that out – briosheje Feb 28 '19 at 11:24

2 Answers2

2

The argument to catch is the callback function, not the error.

You are looking for

Promise.prototype.catch = (function(originalCatch) {
    return function(onRejected) {
        console.log('> > > > > > called .catch on %o with arguments: %o', this, arguments);
        return originalCatch.call(this, error => {
            if (typeof newrelic !== 'undefined') {
                newrelic.noticeError(error);
            } else {
                console.error(error);
            }
            return onRejected(error);
       });
    };
})(Promise.prototype.catch);

Btw, I would recommend to avoid meddling with the Promise.prototype. Intercepting every catch call will get you quite some false positives (that you actually didn't want to log) as well as false negatives (that you should have caught) because the error handler was installed using then or no catch was invoked at all. Better be explicit about where you want errors to go to monitoring, with a simple reusable

function monitorError(error) {
    if (typeof newrelic !== 'undefined') {
        newrelic.noticeError(error);
    } else {
        console.error(error);
    }
}

that you can explicitly inject into or append to promise chains with a simple

.catch(monitorError)
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • is `newrelic.noticeError(arguments[0]);` the same as `newrelic.noticeError(error);` in that context? That's a very interesting approach. – briosheje Feb 28 '19 at 11:27
  • 1
    No, it's not. I think he just forgot to exchange it. ;) – Sebsemillia Feb 28 '19 at 11:28
  • @briosheje It's not the same because an arrow function doesn't have it's own `arguments`. Indeed I just forgot to change it – Bergi Feb 28 '19 at 11:32
  • 1
    Thx for your help. I eventually ended up using your recommendation since I wasn't aware of the possible side effects of meddling of the Promise prototype. But I'll accept @briosheje answer, since he answered the original question as first. I hope that's ok for you. – Sebsemillia Feb 28 '19 at 12:27
2

You can directly invoke your callback through the Promise itself, so that the argument will be evaluated:

(function (Promise) {
    const originalCatch = Promise.prototype.catch;

    Promise.prototype.catch = function () {
        console.log('> > > > > > called .catch on %o with arguments: %o', this, arguments);

        if (typeof newrelic !== 'undefined') {
            originalCatch.apply(this, [newrelic.noticeError]);
               //^--- changed here.
        } else {
            console.error(arguments);
        }

        return originalCatch.apply(this, arguments);
    };
})(Promise);

Sample working code: https://jsfiddle.net/7qgjr6fw/3/

briosheje
  • 7,356
  • 2
  • 32
  • 54