New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Middleware, next() and promises? #409

Closed
tthew opened this Issue Jun 13, 2013 · 2 comments

Comments

Projects
None yet
1 participant
@tthew

tthew commented Jun 13, 2013

So. I'm experiencing some quirks when attempting to implement some basic middleware for a Restify.js application I'm building with specific regard to next() and promise callbacks.

To express the problem in a generic form:

      var server = restify.createServer({
        name: config.name
      });

Promise resolves:

      server.use(function checkAcl(req, res, next) {
        // Promise is resolved
        var promise = function () {
          var deferred = require('q').defer();
          deferred.resolve();
          return deferred.promise;
        }

        promise()
          .then(function () {
            next(); // doesn't get 'called', no response sent, connection eventually times out
          }, function () {
            res.send(new restify.NotAuthorizedError());
          });
      });
      server.use(restify.bodyParser());
      ...

Promise is rejected

      server.use(function checkAcl(req, res, next) {
        // Promise is rejected
        var promise = function () {
          var deferred = require('q').defer();
          deferred.reject();
          return deferred.promise;
        }

        promise()
          .then(function () {
            next(); 
          }, function () {
            res.send(new restify.NotAuthorizedError()); // this works fine
          });
        }
      });
      server.use(restify.bodyParser());      
      ...

Am I doing anything obviously wrong? Any insight? It certainly seems to been related to the promise callbacks, are they somehow suppressing the call to next()? Is this a bug?

@tthew

This comment has been minimized.

Show comment
Hide comment
@tthew

tthew Jun 13, 2013

Follow up. I finally got to the bottom of this. It's down to the order in which middleware is added to the stack with specific regard to the bodyParser(). Adding it prior to the custom middleware produces the expected behaviour.

e.g.

Adding the restify.bodyParser() middleware BEFORE the custom middleware resolves the issue.

n.b:

  // Add bodyParser() to the stack before the custom middleware
  server.use(restify.bodyParser()); 

  server.use(function (req, res, next) {
    // Promise is resolved
    var promise = function () {
      var deferred = require('q').defer();
      deferred.resolve();
      return deferred.promise;
    }

    promise()
      .then(function () {
        next(); // next() now behaves as expected.
      }, function () {
        res.send(new restify.NotAuthorizedError());
      });
  });
  ...

tthew commented Jun 13, 2013

Follow up. I finally got to the bottom of this. It's down to the order in which middleware is added to the stack with specific regard to the bodyParser(). Adding it prior to the custom middleware produces the expected behaviour.

e.g.

Adding the restify.bodyParser() middleware BEFORE the custom middleware resolves the issue.

n.b:

  // Add bodyParser() to the stack before the custom middleware
  server.use(restify.bodyParser()); 

  server.use(function (req, res, next) {
    // Promise is resolved
    var promise = function () {
      var deferred = require('q').defer();
      deferred.resolve();
      return deferred.promise;
    }

    promise()
      .then(function () {
        next(); // next() now behaves as expected.
      }, function () {
        res.send(new restify.NotAuthorizedError());
      });
  });
  ...
@tthew

This comment has been minimized.

Show comment
Hide comment
@tthew

tthew Jun 13, 2013

Further discovered this has been reported before

tthew commented Jun 13, 2013

Further discovered this has been reported before

@tthew tthew closed this Jun 13, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment