Correct way of handling promisses and server respo

问题:

I am trying to improve my code in node.js / sail.js and I am fighting server response in promisses.

When you look at the first .then function you can see that method returns false in case of forbidden access or notFound. Then, in the next .then functions I must check if the return type is === false to skip to section and avoid sending http headers twice. Can this be improved somehow, to skip all next .then methods in case of failure? I can throw an Exception to go in the last .catch but then there must be a case to switch between all possible states. (i.e. forbidden, serverError or even not found)

Notification.findOne({id: req.param('id')})
  .then(function(notification) {
    if (!notification) {
      res.notFound();
      return false;
    }

    if (notification.triggeredBy != req.session.user.id) {
      res.forbidden();
      return false;
    }

    return notification;
  })
  .then(function(notification) {
    if (notification === false) {
      return false;
    }

    return Notification.update(notification.id, actionUtil.parseValues(req));
  })
  .then(function(notification) {
    if (notification === false) {
      return false;
    }

    res.json(notification);
  })
  .catch(function(err) {
    sails.log(err);
    res.serverError({message: 'A server error occurred.'});
  })

回答1:

If I would do this, first I seperate logic and receving/sending function. Second I specify listing of error codes. And it will be like that:

NotificationService.js

/*
 Listing of error codes: {
  * [1] Object not found
  * [2] Forbidden
  * [3] Server error
 }
 */
module.exports = {
    nameOfMethod: function(ID, sessionID) {

        return new Promise(function(resolve, reject) {
            Notification.findOne({ id: ID })
                .then(function(notification) {
                    if (!notification) return reject({ error_code: 1 });
                    if (notification.triggeredBy !== sessionID) return reject({ error_code: 2 });

                    Notification.update(notification.id, actionUtil.parseValues(req))
                        .then(function(notification) {
                            return resolve(notification); // finally return our notification
                        })
                        .catch(function(err) {
                            sails.log.error(err); // It's good when log is classified. In this case is error
                            return reject({ message: 'A server error occurred.' }); 
                        });
                })
                .catch(function(err) {
                    sails.log.error(err);
                    return reject({ message: 'A server error occurred.' });
                });
        });
    }
};

NotificationController.js

module.exports = {
  notifyMe: function(req, res) {
    const ID = req.param('id'), sessionID = req.session.user.id;

    NotificationService.nameOfMethod(ID, sessionID)
      .then(function(notification) {
        return res.send(notification);
      })
      .catch(function(err) {
        switch (err.error_code) {
          case 1:
            return res.notFound(err);

          case 2:
            return res.forbidden(err);

          default:
            return res.serverError(err);
        }
      });
  }
};

In case where I use switch I think it is better way to select right response but on this time I haven't any idea

回答2:

See how filtered .catch() is implemented in Bluebird - it can be useful in your case to throw all errors you need but avoid having a big switch/case block in the catch handler:

.catch(
    class ErrorClass|function(any error)|Object predicate...,
    function(any error) handler
) -> Promise
.caught(
    class ErrorClass|function(any error)|Object predicate...,
    function(any error) handler
) -> Promise

This is an extension to .catch to work more like catch-clauses in languages like Java or C#. Instead of manually checking instanceof or .name === "SomeError", you may specify a number of error constructors which are eligible for this catch handler. The catch handler that is first met that has eligible constructors specified, is the one that will be called.

Example:

somePromise.then(function() {
    return a.b.c.d();
}).catch(TypeError, function(e) {
    //If it is a TypeError, will end up here because
    //it is a type error to reference property of undefined
}).catch(ReferenceError, function(e) {
    //Will end up here if a was never declared at all
}).catch(function(e) {
    //Generic catch-the rest, error wasn't TypeError nor
    //ReferenceError
});

See: http://bluebirdjs.com/docs/api/catch.html#filtered-catch

Instead of:

return false;

you can use:

return Promise.reject(someReason);

or:

throw someReason;

and you won't have to check for those false values - just use (possibly multiple) catch handlers.