0

I want to write a function used to wrap other functions so all errors are caught, including errors generated by a Promise rejection (which normally requires the .catch Promise method).

The goal is to be able to wrap functions so all runtime errors are handled. An example usage is a function we want to run, but that is optional and not part of the core business flow. If there is an error, we want to report it and later fix it, but we do not want it to stop program flow.

It should be able to wrap functions with any number of arguments, and return the same value as the original function, including if the original function returns a promise.

Am I handling all possible cases below? Is there a simpler way to do this?

const catchAllErrors = (fn) => (...args) => {
  try {
    const possiblePromise = fn(...args);

    // Is it a promise type object? Can't use "instanceof Promise" because just
    // for example, Bluebird promise library is not an instance of Promise.
    if (typeof possiblePromise.catch === 'function') {
      return Promise.resolve(possiblePromise).catch((error) => {
        console.log('Caught promise error.', error);
      });
    }

    return possiblePromise;

  } catch (error) {
    console.log('Caught error.', error);
  }
};

// EXAMPLE USAGE

// Applying the wrapper to various types of functions:

const throwsErr = catchAllErrors((x, y) => {
  throw `Error 1 with args ${x}, ${y}.`;
});

const promiseErr = catchAllErrors((a, b) => Promise.reject(`Error 2 with args ${a}, ${b}.`));

const noError = catchAllErrors((name) => `Hi there ${name}.`);

const noErrorPromise = catchAllErrors((wish) => Promise.resolve(`I wish for ${wish}.`));

// Running the wrapped functions:

console.log(throwsErr(1, 2));

promiseErr(3, 4).then((result) => console.log(result));

console.log(noError('folks'));

noErrorPromise('sun').then((result) => console.log(result));
Todd Chaffee
  • 6,754
  • 32
  • 41

1 Answers1

1

Don't try to detect whether something is a promise or not yourself. Use the builtin thenable detection of promise resolution. You can use either the Promise constructor to catch the exception:

const catchAllErrors = (fn) => (...args) => {
  return new Promise(resolve => {
    resolve(fn(...args));
  }).catch((error) => {
    console.log('Caught error.', error);
  });
};

or just go for async/await syntax:

const catchAllErrors = (fn) => async (...args) => {
  try {
    return await fn(...args);
  } catch (error) {
    console.log('Caught error.', error);
  }
};

(If you used Bluebird anyway, you could also call its Promise.try method for this purpose)

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • It doesn't look like your code is returning the value of the original function being wrapped? And in my code I only return a promise if the wrapped function returns a promise. – Todd Chaffee Dec 14 '19 at 17:27
  • I'm not necessarily using Bluebird. Just want it to work for Bluebird too. Thanks for that tip though. – Todd Chaffee Dec 14 '19 at 17:28
  • @ToddChaffee I didn't think you needed a result back, as when there's an error, you get `undefined` only anyway? But sure, you could `return` the promise, if you really wanted to, but I think in those cases the caller should handle errors themselves (and also know whether the function they are calling is asynchronous or not). – Bergi Dec 14 '19 at 17:34
  • I need the result back, as I've done in my implementation. There isn't always an error. I'll add an example for that in my code. Our requirement is that the caller should not have to handle the errors themselves and that any type of function can be wrapped. We don't care if the function is async, we just let it execute. If it is async, even better so it doesn't slow down the core flow. But it does not have to be. – Todd Chaffee Dec 14 '19 at 17:42
  • @ToddChaffee But if the caller does use result, he *does* always need to handle the absence of a result (in case of an error), there's no way around that. Can you show some usage examples where you need the result back? – Bergi Dec 14 '19 at 17:46
  • I modified my examples and added two more to show using the result. It's fine if the result is `undefined` when there is an error. – Todd Chaffee Dec 14 '19 at 17:55
  • 1
    @ToddChaffee That sounds still weird to me if you're fine with `undefined` in some cases but not in all cases… I would then rather put the `console.log` inside the `cathAllErrors` callback (where it might not be called at all in case of an exception). But if you really have this requirement, your original code is probably fine, only thing I'd change is to check for `.then` instead of `.catch` to be a function. – Bergi Dec 14 '19 at 17:59
  • Thanks for your help and advice. Why check `then` instead of `catch`? Any reading I could do on that? – Todd Chaffee Dec 14 '19 at 18:02
  • 1
    @ToddChaffee Because [that's the check that `Promise.resolve` does itself](https://stackoverflow.com/a/29437927/1048572). `catch(…)` is meaningless anyway, it's just an alias for `then(undefined, …)` – Bergi Dec 14 '19 at 18:12