return values of next()

[refractalize]

I use promises a lot in my express apps, and I was curious about how to handle errors from routes that used promises in a more general way. What I wanted to do was install some middleware that could handle any error from any route below it, whether it was a normal error thrown or an asynchronous error in the form of a rejected promise.

This is what I wanted to do:

function handleErrors(req, res, next) {
  function sendError(error) {
    console.log(error);
    res.status(500).send({message: error.message});
  }

  var result;
  try {
    result = next();
  } catch (error) {
    sendError(error);
  }

  if (result && typeof result.then === 'function') {
    result.then(undefined, sendError);
  }
}

app.use(handleErrors);

// one of many routes
app.get('/thing', function (req, res) {
  return mongoDb.find({_id: 'thing'}).exec().then(function (thing) {
    res.send(thing);
  });
});

Now, if something happens to the mongo DB, the /thing route will NOT res.send() anything, simply return the promise to the error handling middleware, which will detect the error and all is (kinda) ok. At least the server returns something.

Problem is that express and connect don't really think about returning the result of next() functions. This is, on the surface at least, fairly easy to fix. I've made a start in the files below. But the connect API is very well understood already, and while this is not a breaking change, it is a very subtle extension to the API.

I've started putting return statements into express in the parts that matter, just to get this example working. I haven't finished yet, I intend to write tests and explore the wider scope of this change.

Is appetite for this?

(and I really don't mean to start an argument about promises vs callbacks vs generators)

[felixfbecker]

If express 5 supports promises this will be added earlier or later as next() will need to return a promise.

[refractalize]

Cheers, happy to finish this off when I get a nod. What are the ideas for express 5 and promises, anything you can point me to?

[felixfbecker]

#2237 #2259 #2763 pillarjs/router#25
But I think there hasn't been any discussion yet about return values of promises.

[dougwilson]

So, if this PR is purely a way to get Promises through in Express, I think the effort belongs in other initiatives, rather than trying to get return values from next passed up. Saying that next() can return a value from an upper middleware will be a large community effort, so needs to be carefully thought about (there are many, many middleware-wrapping patterns that do not pass up return values people will continue to copy-and-paste for years to come, as well as just all modules wrapping will have to deal with).

On the surface, it seems reasonable, but unfortunately I'm going to have to decline this after some behind the scenes talking. The reason is that next() is just a callback, and callbacks do not return a value for a very good reason: they are executed after the current stack has unraveled:

app.use(function (req, res, next) {
  // do things
  setImmediate(function () { // an async thing
    next() // impossible to return any result here
  })
})

Your pattern above will break down as soon as you use just one async middleware from npm between app.use(handleErrors); and your app.get handler, as they will not propagate any values, and they may never, if the author doesn't want to use promises in their library.

In a future promise Express (expected to be 5.0), your given code below will work as you expect:

app.get('/thing', function (req, res) {
  return mongoDb.find({_id: 'thing'}).exec().then(function (thing) {
    res.send(thing);
  });
});

Your handleErrors may not, but that would probably warrant a different discussion, around how promises should interact with prior middlewares and if next() should just always return a promise, even if a later middleware does not use promises (i.e. should Express force everything into a promise?).

[felixfbecker]

Good reasoning @dougwilson. I initally thought by return value he meant a callback return value, like calling next(err) for an error and next(null, value) for a value.