Regression: Error with express and compression module with version >= 1.6.0

[dmmalam]

express-session gives the following error when used with the expressjs/compression module from version 1.6.0. I've stuck to using 1.5.2 to avoid the error.

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: write after end
    at writeAfterEnd (_stream_writable.js:132:12)
    at Gzip.Writable.write (_stream_writable.js:180:5)
    at ServerResponse.res.write (/Users/dmalam/Code/q/node_modules/compression/index.js:93:18)
    at ServerResponse.res.end (/Users/dmalam/Code/q/node_modules/express-session/index.js:212:20)
    at Gzip.<anonymous> (/Users/dmalam/Code/q/node_modules/compression/index.js:208:13)
    at Gzip.emit (events.js:117:20)
    at _stream_readable.js:938:16
    at process._tickCallback (node.js:419:13)

[dougwilson]

Hm. I'mm check it out. In the meantime, especially if I can't seem to figure it out, can you post a tiny app that produces the error?

[joewagner]

Here's a tiny app that I think demonstrates the issue

var express = require('express')
var app = express()
var session = require('express-session');
var compression = require('compression');

app.use(session({secret: 'keyboard cat'}));
app.use(compression());

app.get('/', function (req, res, next) {
    res.setHeader('Content-Type', 'text/plain')
    res.write('hello, world\n');
    res.end();
});

app.listen(3000);

then do

curl localhost:3000 -v -H 'Accept-Encoding: gzip'

express-session's call to [res.write] is called after the gzip stream is closed because of the order in which the middleware is wrapping res.end. If you use compression before express-session it seems to work

[dougwilson]

Interesting. @joewagner you are the man :) I didn't have time to look yet, so I think it's far to say that is what caused the issue for @dmmalam

1.6.0 did change the res.end patches, so I will need to take a closer look into what is going on there. Darn you patching!

[dmmalam]

Reordering has seemed to fix the issue for now. Thanks @joewagner

Dharmesh Malam

On Mon, Aug 4, 2014 at 11:00 PM, Douglas Christopher Wilson <
[email protected]> wrote:

Interesting. @joewagner https://github.com/JoeWagner you are the man :)
I didn't have time to look yet, so I think it's far to say that is what
caused the issue for @dmmalam https://github.com/dmmalam

1.6.0 did change the res.end patches, so I will need to take a closer
look into what is going on there. Darn you patching!

—
Reply to this email directly or view it on GitHub
#72 (comment).

[dougwilson]

Reordering has seemed to fix the issue for now

Great for the confirmation. We'll leave this open, though, because I think it's definitely a bug, in one library or the other, so this will remind me.

[joewagner]

@dougwilson I looked at this a bit more, and looks like the fix for issue expressjs/compression#61 introduced this issue.
I'm trying to think about how to fix and it seems either change compression to check if zlib stream is ended before trying to write to the stream, or git rid of the calls to res.write() inside of the patch for res.end.
Maybe both, or maybe neither! :D

[dougwilson]

Maybe both, or maybe neither! :D

haha. Yep. I have looked into it a little bit and I think I'm leaning towards both things need fixing and I need to really get out the expressjs/discussions#301 so it can truly be fixed in both places by a real solution, but that work is delayed right now due to express chopping (the timeline for turning express into a bunch of micro libs has sped up).

[joewagner]

that work is delayed right now due to express chopping (the timeline for turning express into a bunch of micro libs has sped up).

Totally! let me know if you want me to try to come up with PR for either of these

[dougwilson]

let me know if you want me to try to come up with PR for either of these

It's always welcome :)

[dougwilson]

OK, working on a fix now. It's really a bug in this module, because it changes the meaning of res.end. I think there is a better way to return the correct value from res.end but keep the module working in the same way for compatibility.

[dougwilson]

You can also use 1.7.3 of this module if you want the old middleware order you had.