Skip to content

Commit

Permalink
fix back-pressure behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
dougwilson committed Jun 1, 2014
1 parent b0aeb7e commit 5426d13
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 4 deletions.
5 changes: 5 additions & 0 deletions HISTORY.md
@@ -1,3 +1,8 @@
unreleased
==========

* fix back-pressure behavior

1.0.3 / 2014-05-29
==================

Expand Down
19 changes: 15 additions & 4 deletions index.js
Expand Up @@ -57,7 +57,8 @@ module.exports = function compression(options) {

return function compression(req, res, next){
var write = res.write
, end = res.end
var on = res.on
var end = res.end
, compress = true
, stream;

Expand Down Expand Up @@ -97,6 +98,14 @@ module.exports = function compression(options) {
: end.call(res);
};

res.on = function(type, listener){
if (!stream || type !== 'drain') {
return on.call(this, type, listener)
}

return stream.on(type, listener)
}

onHeaders(res, function(){
// default request filter
if (!filter(req, res)) return;
Expand Down Expand Up @@ -140,15 +149,17 @@ module.exports = function compression(options) {

// compression
stream.on('data', function(chunk){
write.call(res, chunk);
if (write.call(res, chunk) === false) {
stream.pause()
}
});

stream.on('end', function(){
end.call(res);
});

stream.on('drain', function() {
res.emit('drain');
on.call(res, 'drain', function() {
stream.resume()
});
});

Expand Down
51 changes: 51 additions & 0 deletions test/test.js
@@ -1,3 +1,4 @@
var crypto = require('crypto');
var http = require('http');
var request = require('supertest');
var should = require('should');
Expand Down Expand Up @@ -133,6 +134,55 @@ describe('compress()', function(){
})
})

it('should back-pressure', function(done){
var buf
var client
var resp
var server = createServer({ threshold: 0 }, function (req, res) {
resp = res
res.setHeader('Content-Type', 'text/plain')
res.write('start')
pressure()
})
var wait = 2

crypto.pseudoRandomBytes(1024 * 128, function(err, chunk){
buf = chunk
pressure()
})

function complete(){
if (--wait === 0) done()
}

function pressure(){
if (!buf || !resp || !client) return

while (resp.write(buf) !== false) {
resp.flush()
}

resp.on('drain', function(){
resp.write('end')
resp.end()
})
resp.on('finish', complete)
client.resume()
}

request(server)
.get('/')
.request()
.on('response', function (res) {
client = res
res.headers['content-encoding'].should.equal('gzip')
res.pause()
res.on('end', complete)
pressure()
})
.end()
})

describe('threshold', function(){
it('should not compress responses below the threshold size', function(done){
var server = createServer({ threshold: '1kb' }, function (req, res) {
Expand Down Expand Up @@ -222,6 +272,7 @@ describe('compress()', function(){
res.statusCode = typeof res.flush === 'function'
? 200
: 500
res.flush()
res.end()
})

Expand Down

6 comments on commit 5426d13

@robbywashere-zz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am running drywall https://github.com/jedireza/drywall - and after a few hours of debugging I have narrowed down a bug to this commit of this module. The request would simply hang until the timeout reached its maximum by the browser.

@dougwilson
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @robbywashere . Please open an issue rather than commenting on a commit for better visibility.

@robbywashere-zz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

@steida
Copy link

@steida steida commented on 5426d13 Jun 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robbywashere-zz
Copy link

@robbywashere-zz robbywashere-zz commented on 5426d13 Jun 4, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dougwilson
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi everyone who has been commenting on this commit. The issue should be fixed in version 1.0.5 on npm.

Please sign in to comment.