3

I am working in a new codebase that has a pattern where some reusable functions return a promise from a chain, but they don't handle errors.

Here is an example:

function createScheduleForGroup(group) {
  if (notInProduction()) {
    group.gschedScheduleId = '0';
    return Bluebird.resolve(group);
  }
// note: schedule is a global within the file
  return schedule.createSchedule(group.name).then(function (schedule) {
    group.gschedScheduleId = schedule.id;
    return group.save();
  });
}

In the example above, there isn't a .catch or a reject function passed to the .then.

The function is eventually used in an express route where the error is handled:

router.post('/schedule', function(req, res, next) {
       scheduleLogical
         .createScheduleGroup(req[config.entity])
         .then(function(group) {
           res.status(201).json(group);
         })
         .catch(next); 

// if creteScheduleGroup throws an error it is handled here

Is it a common pattern to not define error handlers for a promise returned from a function, and anticipate whoever uses the function to attach the appropriate error handlers?

To make this clearer for my own understanding, I made a simulation of this specific function and all the functions used within the promise chain. Here it is:

function getScheduleMock() {
  // this promise promisifys an older api that uses callbacks
  return new Promise((resolve, reject) => {
    // note this is simulating an api call:
    const response = Math.round(Math.random());
    // 0 === err, 1 === success

    if(response === 0) return reject(response);
    resolve(response);
    
  })
  // error hanlding :)
  .catch(err => {
    console.log(err);
    return Promise.reject(err);
    // there is an error handling function that logs the errors. If the error doesn't match expected errors, it rejects the error again as I have here.
  })
  .then(responseData => {
    return Promise.resolve(responseData);
  })
}

function createScheduleForGroupMock() {
  return getScheduleMock().then(responseData => Promise.resolve('everything worked :)'));
 // Note: group.save() from the original returns a promise
  // just like the actual example, the promise is returned
  // but there isn't any form of error handling except within the getScheduleMock function
}

createScheduleForGroupMock(); // when the error is rejected in the .catch() in getScheduleMock, the error is unhandled.

/* ***************** */


/* the createScheduleForGroup method is used within an express route which has error handling, here is an example of the code: */

// router.post('/schedule', function(req, res, next) {
//       scheduleLogical
//         .createScheduleGroup(req[config.entity])
//         .then(function(group) {
//           res.status(201).json(group);
//         })
//         .catch(next); 
        
// if creteScheduleGroup throws an error it is handled here

I'm fairly new to error handling promises and from what I've been reading and practicing, I generally felt you should always include an error handler. The codebase I'm working in has a lot of methods that use the pattern of createScheduleForGroup where there isn't an error handler defined in the function, but instead, it is handled and attached after the function is used.

Some of the functions used within createScheduleForGroup handle their own errors, and I'm confused and curious about the balance on when to handle errors in a function that returns a promise and when to attach them when you use the function.

HelloWorld
  • 10,529
  • 10
  • 31
  • 50
  • 2
    Promise error handling isn't any different from conventional error handling, you should only `catch` when you can gracefully handle the error, in all other cases error handling should be deferred to the calling method. – Jake Holzinger Feb 26 '19 at 19:09
  • I will also add that catch-log-rethrow is an anti-pattern. Ideally you would have the error bubble all the way up to a handler that logs errors before bringing down and restarting the application, or log the error when it is handled (if necessary). – Jake Holzinger Feb 26 '19 at 19:16

2 Answers2

3

Is it a common pattern to not define error handlers for a promise returned from a function, and expect whoever uses the function to attach the appropriate error handlers?

Yes, totally. It's not just "a common pattern", it's the absolute standard pattern.

Just like you don't put a try/catch statement in every synchronous function, you don't put a .catch callback on every promise that you return. In fact, it's considered an antipattern to catch errors that you cannot handle.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
-1

You can have an error handler in each function.

function aPromise() {

   return new Promise(function(resolver, reject){

      //Handle any error here and attach different information via your own errror class

   })

}

async function parentProcess() {

   try{
    await aPromise()
   }
   catch(e) {
       //Handle and attach more info here
   }
}

function grandParentProcess() {

   try{
     parentProcess();
   }
   catch(e){
       //Handle the error
   }

}

You don't essentially have to handle error in the parent function if the grand parent function handles it to avoid the UnhandledPromiseRejection error.

Charlie
  • 22,886
  • 11
  • 59
  • 90
  • thanks! I think my main question is when working on a bigger codebase, is there a best practice when creating functions that return promises? I understand the exception can be handled if the grandparent function handles it, but what happens if there isn't a grandparent function? As you mentioned, there may be an `uncaughtExceptionHandler`, but it feels unnatural to me to depend on it, from my understanding it feels like the `uncaughExceptionHandler` is there in case an error is thrown that isn't handled. It's highly possibly my perception on this is 100% wrong. – HelloWorld Feb 26 '19 at 19:11