Writing Clean Code With Nested Promises

问题:

I'm writing an app that talks to Apple to verifyReceipts. They have both a sandbox and production url that you can post to.

When communicating with Apple, if you receive a 21007 status, it means you were posting to the production url, when you should be posting to the sandbox one.

So I wrote some code to facilitate the retry logic. Here's a simplified version of my code:

var request = require('request')
  , Q = require('q')
  ;

var postToService = function(data, url) {
  var deferred = Q.defer();
  var options = {
    data: data,
    url: url
  };

  request.post(options, function(err, response, body) {
    if (err) { 
      deferred.reject(err);
    } else if (hasErrors(response)) {
      deferred.reject(response);
    } else {
      deferred.resolve(body);
    }
  });

  return deferred.promise;
};

exports.verify = function(data) {
  var deferred = Q.defer();

  postToService(data, "https://production-url.com")
    .then(function(body) {
      deferred.resolve(body);
    })
    .fail(function(err) {
      if (err.code === 21007) {
        postToService(data, "https://sandbox-url.com")
          .then(function(body){
            deferred.resolve(body);
          })
          .fail(function(err) {
            deferred.reject(err);
          });
      } else {
        deferred.reject(err);
      }

    });

  return deferred.promise;
};

The retry portion in the verify function is pretty ugly and difficult to read with the nested promises. Is there a better way of doing this?

回答1:

You can re-throw an error in the rejection handler to continue rejecting the promise, or you can return a new promise to replace the rejection.

exports.verify = function(data) {
  return postToService(data, "https://production-url.com")
    .fail(function(err) {
      if (err.code === 21007) {
        return postToService(data, "https://sandbox-url.com")
      } else {
        throw err
      }
    });
};

回答2:

Here are a couple of possibilities. Because this question has an element of personal taste to it, you may or may not like what you see!

(Admission - I have not tested this code)

Option 1 - Use a wrapper for resolve and reject. This adds 'noise' in the form of the helper functions, but tidies up the rest.

var resolve = function (deferred, ob) {
  return function () {
    deferred.resolve(ob);
  };
};

var reject = function (deferred, ob) {
  return function () {
    deferred.reject(ob);
  };
};

exports.verify = function(data) {
  var deferred = Q.defer();

  postToService(data, "https://production-url.com")
    .then(resolve(deferred, body))
    .fail(function(err) {
      if (err.code === 21007) {
        postToService(data, "https://sandbox-url.com")
          .then(resolve(deferred, body))
          .fail(reject(deferred, err));
      } else {
        deferred.reject(err);
      }
    });

  return deferred.promise;
};

Option 2 - Use bind. This has the advantage of using existing JS functionality, but you have duplicate references to deferred when creating the callbacks.

exports.verify = function(data) {
  var deferred = Q.defer();

  postToService(data, "https://production-url.com")
    .then(deferred.resolve.bind(deferred, body))
    .fail(function(err) {
      if (err.code === 21007) {
        postToService(data, "https://sandbox-url.com")
          .then(deferred.resolve.bind(deferred, body))
          .fail(deferred.reject.bind(deferred, err));
      } else {
        deferred.reject(err);
      }
    });

  return deferred.promise;
};

Option 3 - Use bind and 'method handles' (minor variation on #2).

exports.verify = function(data) {
  var deferred = Q.defer();
  var resolve = deferred.resolve;
  var reject = deferred.reject;

  postToService(data, "https://production-url.com")
    .then(resolve.bind(deferred, body))
    .fail(function(err) {
      if (err.code === 21007) {
        postToService(data, "https://sandbox-url.com")
          .then(resolve.bind(deferred, body))
          .fail(reject.bind(deferred, err));
      } else {
        deferred.reject(err);
      }
    });

  return deferred.promise;
};

Option 4 - Monkey patch deferred.

function patch(deferred) {
  deferred.resolveFn = function (ob) {
    return function () {
      deferred.resolve(ob);
    };
  };
  deferred.rejectFn = function (ob) {
    return function () {
      deferred.reject(ob);
    };
  };
  return deferred;
}

exports.verify = function(data) {
  var deferred = patch(Q.defer());

  postToService(data, "https://production-url.com")
    .then(deferred.resolveFn(body))
    .fail(function(err) {
      if (err.code === 21007) {
        postToService(data, "https://sandbox-url.com")
          .then(deferred.resolveFn(body))
          .fail(deferred.rejectFn(err));
      } else {
        deferred.reject(err);
      }
    });

  return deferred.promise;
};

回答3:

You might consider something like the following. I think judicious use of whitespace can help readability. You'll probably want to find a reasonable style standard that your team feels good about and stick with it!

exports.verify = function(data) {
  var deferred = Q.defer();

  postToService(data, "https://production-url.com")

    .then(deferred.resolve, function(err) {

      if (err.code === 21007) {

        postToService(data, "https://sandbox-url.com")

          .then(deferred.resolve, deferred.reject);

      } else { deferred.reject(err); }

    });

 return deferred.promise;
};

回答4:

Stuart's answer is right, the point was to chain promises. and I would like to clarify that it is not needed to use Q.defer just for wrapping. it's even considered an anti-pattern. See reasons here The Deferred anti-pattern

var request = require('request')
    , Q = require('q');

var PRODUCTION_URL = "https://production-url.com",
var SANDBOX_URL    = "https://sandbox-url.com",


export.verify = function() {

  return postToProduction(data)
         .fail( function(error) {
             if (error.code === 21007 ) return postToSanbox(data);
             throw error;
         });
}

function postToProduction(data) {
    return postToService(data, PRODUCTION_URL);
}

function postToSandbox(data) {
    return postToService(data, SANDBOX_URL);
}

function postToService(data, url) {
   var deferred = Q.defer();

   var options = {
      data: data,
      url: url
   };

  request.post(options, function(err, response, body) {
    if (err) return deferred.reject(err);
    if (hasErrors(response)) return deferred.reject(response);

    deferred.resolve(body);    
  });

  return deferred.promise;   
}