Achieve error differentiation in Promises

Question

Background

I have a REST API using MongoDB, Node.js and Express that makes a request to my NoSQL DB and depending on different results, I want to differentiate which error I send the customer.

Problem

The current version of my code has a generic error handler and always sends the same error message to the client:

api.post("/Surveys/", (req, res) => {
        const surveyJSON = req.body;
        const sender = replyFactory(res);

        Survey.findOne({_id: surveyJSON.id})
            .then(doc => {
                if(doc !== null)
                    throw {reason: "ObjectRepeated"};

                //do stuff
                return new Survey(surveyJSON).save();
            })
            .then(() => sender.replySuccess("Object saved with success!")) 
            .catch(error => {
                /*
                 * Here I don't know if:
                 * 1. The object is repeated
                 * 2. There was an error while saving (eg. validation failed)
                 * 3. Server had a hiccup (500)
                 */
                sender.replyBadRequest(error);    
            });
    });

This is a problem, because the client will always get the same error message, no matter what and I need error differentiation!

Research

I found a possible solution, based on the division of logic and error/response handling:

• https://stackoverflow.com/questions/26076511/handling-multiple-catches-in-promise-chain

However, I don't understand a few things:

1. I don't see how, at least in my example, I can separate the logic from the response. The response will depend on the logic after all!
2. I would like to avoid error sub-classing and hierarchy. First because I don't use bluebird, and I can't subclass the error class the answer suggests, and second because I don't want my code with a billion different error classes with brittle hierarchies that will change in the future.

My idea, that I don't really like either

With this structure, if I want error differentiation, the only thing I can do is to detect an error occurred, build an object with that information and then throw it:

.then(doc => {
    if(doc === null)
        throw {reason: "ObjectNotFound"};

    //do stuff
    return doc.save();
})
.catch(error => {
    if(error.reason === "ObjectNotFound")  
        sendJsonResponse(res, 404, err);
    else if(error.reason === "Something else ")
        sendJsonResponse(/*you get the idea*/);
    else //if we don't know the reasons, its because the server likely crashed
        sendJsonResponse(res, 500, err);
});

I personally don't find this solution particularly attractive because it means I will have a huge if then else chain of statements in my catch block.

Also, as mentioned in the previous post, generic error handlers are usually frowned upon (and for a good reason imo).

Questions

How can I improve this code?

Answer 1

Would it help to rewrite it along the lines of:

function notFoundErrorHandler(error) {
  if(error.reason === "ObjectNotFound")  
      return {code: 404, error:err);
}
function someOtherErrorHandler(error) {
  if(error.reason === something else")  
      return {code: 499, error:err);
}
function serverErrorHandler(error) {
  // No if because this is a catch all
  return {code: 500, error:err);
}

let handlers = [notFoundErrorHandler,
                someOtherErrorHandler,
                serverErrorHandler     ];

Survey.findOne({_id: surveyJSON.id})
  ...
  .catch(error => {
    respondToError(error, handlers);
  } );

function respondToError(error, handlers) {
  for (let handler of handlers) {
    let {code, error} = handler(error);
    if (code) {
       sendJsonResponse(res, code, error);
       return;
    }
  }
  console.error("Shouldn't get here");
}

Answer 2

Objectives

When I started this thread, I had two objectives in mind:

1. Having error differentiation
2. Avoid an if then else of doom in a generic catcher

I have now come up with two radically distinct solutions, which I now post here, for future reference.

Solution 1: Generic error handler with Errors Object

This solution is based on the solution from @Marc Rohloff, however, instead of having an array of functions and looping through each one, I have an object with all the errors.

This approach is better because it is faster, and removes the need for the if validation, meaning you actually do less logic:

const errorHandlers = {
    ObjectRepeated: function(error){
        return { code: 400, error };
    },
    SomethingElse: function(error){
        return { code: 499, error };
    }
};

Survey.findOne({
        _id: "bananasId"
    })
    .then(doc => {

        //we dont want to add this object if we already have it
        if (doc !== null)
            throw { reason: "ObjectRepeated", error:"Object could not be inserted because it already exists."};

        //saving empty object for demonstration purposes
        return new Survey({}).save();
    })
    .then(() => console.log("Object saved with success!"))
    .catch(error => {
        respondToError(error);
    });

    const respondToError = error => {
        const errorObj = errorHandlers[error.reason](error);

        if (errorObj !== undefined)
            console.log(`Failed with ${errorObj.code} and reason ${error.reason}: ${JSON.stringify(errorObj)}`);
        else 
            //send default error Obj, server 500
            console.log(`Generic fail message ${JSON.stringify(error)}`);
    };

This solution achieves:

1. Partial error differentiation (I will explain why)
2. Avoids an if then else of doom.

This solution only has partial error differentiation. The reason for this is because you can only differentiate errors that you specifically build, via the throw {reaon: "reasonHere", error: "errorHere"} mechanism.

In this example, you would be able to know if the document already exists, but if there is an error saving the said document (lets say, a validation one) then it would be treated as "Generic" error and thrown as a 500.

To achieve full error differentiation with this, you would have to use the nested Promise anti pattern like the following:

.then(doc => {

    //we dont want to add this object if we already have it
    if (doc !== null)
        throw { reason: "ObjectRepeated", error:"Object could not be inserted because it already exists." };

    //saving empty object for demonstration purposes
    return new Survey({}).save()
        .then(() => {console.log("great success!");})
        .catch(error => {throw {reason: "SomethingElse", error}});
})

It would work... But I see it as a best practice to avoid anti-patterns.

Solution 2: Using ECMA6 Generators via co.

This solution uses Generators via the library co. Meant to replace Promises in near future with a syntax similar to async/await this new feature allows you to write asynchronous code that reads like synchronous (well, almost).

To use it, you first need to install co, or something similar like ogen. I pretty much prefer co, so that is what i will be using here instead.

const requestHandler = function*() {

    const survey = yield Survey.findOne({
        _id: "bananasId"
    });

    if (survey !== null) {
        console.log("use HTTP PUT instead!");
        return;
    }

    try {
        //saving empty object for demonstration purposes
        yield(new Survey({}).save());
        console.log("Saved Successfully !");
        return;
    }
    catch (error) {
        console.log(`Failed to save with error:  ${error}`);
        return;
    }

};

co(requestHandler)
    .then(() => {
        console.log("finished!");
    })
    .catch(console.log);

The generator function requestHandler will yield all Promises to the library, which will resolve them and either return or throw accordingly.

Using this strategy, you effectively code like you were coding synchronous code (except for the use of yield).

I personally prefer this strategy because:

1. Your code is easy to read and it looks synchronous (while still have the advantages of asynchronous code).
2. You do not have to build and throw error objects every where, you can simply send the message immediately.
3. And, you can BREAK the code flow via return. This is not possible in a promise chain, as in those you have to force a throw (many times a meaningless one) and catch it to stop executing.

The generator function will only be executed once passed into the library co, which then returns a Promise, stating if the execution was successful or not.

This solution achieves:

1. error differentiation
2. avoids if then else hell and generalized catchers (although you will use try/catch in your code, and you still have access to a generalized catcher if you need one).

Using generators is, in my opinion, more flexible and makes for easier to read code. Not all cases are cases for generator usage (like mpj suggests in the video) but in this specific case, I believe it to be the best option.

Conclusion

Solution 1: good classical approach to the problem, but has issues inherent to promise chaining. You can overcome some of them by nesting promises, but that is an anti pattern and defeats their purpose.

Solution 2: more versatile, but requires a library and knowledge on how generators work. Furthermore, different libraries will have different behaviors, so you should be aware of that.