Second middleware stack

[rlidwka]

Just sharing my thoughts about res.send() mentioned in https://github.com/visionmedia/express/issues/2249#issuecomment-49604822

This is just an idea, and it has a lot of flaws as is, but maybe somebody would find it useful.

So, right now res.send() terminates middleware stack. next() handler is not called, so no middlewares won't be executed. Well, except for those who monkey-patches res.send() of course, but people usually see it as a hack.

Currently all middlewares are supposed to be executed before result of the request is known.

Here is a typical express application with a few middlewares.

app.use(bodyParser)
app.use(etagify)
app.use(compress)
app.use(function convert_result_to_yaml(req, res, next) {
    // I'm typing this from memory and realizing that I
    // remember this pattern a lot better that I would like to
    var _send = res.send
    res.send = function(data) {
        res.send = _send
        data = YAML.stringify(data)
        res.send(data)
    }
})
app.get('/', function(req, res) {
    res.send({hello: 'world'})
})

Please note that etagify, compress and that weird custom formatter are usually only overriding res.send() and their main work is done after res.send is executed.

And because of the way how they work, etagify is specified before compress, but it is actually called after it.

Idea: res.send(data) could pass the data to a second middleware stack. This stack would know that the result of the request is ready and perform some operations over it.

app.use(bodyParser)
app.get('/', function(req, res) {
    res.send({hello: 'world'})
})
app.final(function convert_result_to_yaml(req, res, data, next) {
    // `data` would be an object/string/whatever res.send() is called with
    //
    // note changed signature of this and next functions
    next(null, YAML.stringify(data))
})
app.final(compress)
app.final(etagify)

Which some people might find more clear.

Note that real-world middlewares are much more complex because of the streaming, and I don't know how to deal with this yet. Well, maybe do res.send(Stream) and make use of transform streams.

[jonathanong]

this is way too complicated and would require rewriting all the middleware. at this point, you might as well use another framework like koa that doesn't have this issue

[tlivings]

Another way to handle it could be through emitting more events. Hapi does something like this, allowing you to write hooks for postResponse for example.

[aredridel]

The other alternative is to allow a middleware to replace req and res, letting them be made into transform streams or other interposed constructs.

[jonathanong]

part of the beauty of express middleware (at least the ones we provide) is that you don't need express to use these middleware. you can create a pyramid of middleware(req, res, function (err) {})s if you'd like and avoid using express all together. or you can use them with connect, restify, etc. hell, use async.series() and bind all the middleware with .bind(this, req, res).

with both suggestions, you're creating a much more opinionated framework with less compatibility trying to solve a problem that shouldn't exist in the first place. the best solution is just to get rid of the problem: callbacks.

i'm also -1 on streams because streams are super broken and it's non-trivial to create a well-written stream utility/middleware.

[jonathanong]

replacing req and res could be a possibility, but I can't think of an implementation that is fast and not prone to edge cases. the last thing you want is creating replacements in every single middleware.

[dougwilson]

I am also generally -1 on this (especially since the example above is trivially implemented by a shared function), but am willing to hear a compelling argument for the added complexity.

[tlivings]

Although the interface for middleware is common I think it's too general an assumption to say all middleware is interoperable.

In any case, the intent of this thread, unless I am mistaken, is to call for control over the response pipeline in a fashion similar to that of middleware.

Personally I think this is a feature lacking in express / connect that is not simply an edge case and could benefit the community greatly.

[dougwilson]

Another way to handle it could be through emitting more events. Hapi does something like this, allowing you to write hooks for postResponse for example.

Node.js core events do not stack, so it would not work for the use-case in the OP.

The other alternative is to allow a middleware to replace req and res, letting them be made into transform streams or other interposed constructs.

This will lead to issues caused by things like unity and Phusion Passenger: they'll implement res/req mostly, but not quite and things would just be broken. The only solution would be to not pass Node.js core req/res at all and instead a completely abstracted version.

I think it's too general an assumption to say all middleware is interoperable.

We are specifically talking about the middleware we provide. They don't even use express/connect in their tests. The current pattern is completely interoperable, though it doesn't mean people have to make their middleware that way (read: they are lazy).

[aredridel]

Replacing req and res would be pretty easy to extend the interface to accommodate:

function (req, res, next) { 
    // do things
    next(req, new ProxyRequest(res).pipe(res));
}

res is a stream with some properties. It's really not hard to emulate, extend or wrap the interface, especially given a working one. I think the only reason Passenger broke them so badly was a poorly managed update to streams2 API. Now we have core modules that do the hard part of streams, the buffering.

[dougwilson]

res is way more than stream just a stream. You have to emulate every single aspect of OutgoingMessage in Node.js HTTP core, like setHeader, getHeader, headersSent, etc.

[aredridel]

Yeah, or delegate to the original. Certainly not the most elegant thing in the universe, but not a collossal hack.

[dougwilson]

The following is the public interface of OutgoingMessage you would need to emulate:

writable property
chunkedEncoding property
sendDate property
finished property
socket property
connection property
statusCode property
headersSent property
setTimeout method
destroy method
setHeader method
getHeader method
removeHeader method
write method
end method
addTrailers method
writeContinue method
writeHead method
writeHeader method
close event
aborted event
error event
end event
timeout event
drain event

I'm sure I'm missing things. This also does not include all the possible signatures to the methods.

[dougwilson]

Also, @aredridel I'm going to extract a utility as a module for the community where you can give a res and a transform stream and the transform stream would be injected into res directly, without changing the res reference and thus working 100% with the current middleware. Wouldn't this work here without all the caveats?

[dougwilson]

The module also forwards back-pressure, etc.

[aredridel]

That'd do the job. Ugly in the same ways, inside-out. Clever, too.

[dougwilson]

@aredridel you're welcome you subscribe to https://github.com/expressjs/discussions/issues/301 to know when/were it lands.

[tlivings]

That would be awesome.

[defunctzombie]

I don't think this is something we should be pushing for right now.

[rlidwka]

Actually, thinking about it... We do have second middleware stack. That's error-handling middlewares.

Are there any side-effects for re-purposing that one for general usage, not just for errors?

I mean, the code above could be rewritten like this:

app.get('/', function(req, res, next) {
    next({hello: 'world'}) // this would be response, not an error
})

app.use(function(body, req, res, next) {
    if (body instanceof Error) return next(body)
    next(YAML.stringify(body))
})

app.use(function(body, req, res, next) {
    if (typeof(body) === 'string') {
        res.header('content-encoding', 'gzip')
        body = gzip(body)
    }
    next(body)
})

... and the funny thing is that it's already working perfectly fine. I wonder why I didn't notice that before. :D