fix: call underlying implementation properly

index.js

@@ -23,6 +23,7 @@ const isFinished = require('on-finished').isFinished
 var onHeaders = require('on-headers')
 var vary = require('vary')
 var zlib = require('zlib')
+var ServerResponse = require('http').ServerResponse
 
 /**
  * Module exports.
@@ -36,6 +37,7 @@ module.exports.filter = shouldCompress
  * @private
  */
 var cacheControlNoTransformRegExp = /(?:^|,)\s*?no-transform\s*?(?:,|$)/
+
 var SUPPORTED_ENCODING = ['br', 'gzip', 'deflate', 'identity']
 var PREFERRED_ENCODING = ['br', 'gzip']
 
@@ -87,23 +89,49 @@ function compression (options) {
 
     // proxy
 
-    res.write = function write (chunk, encoding) {
-      if (ended) {
+    res.write = function write (chunk, encoding, callback) {
+      if (res.destroyed || res.finished || ended) {
+        // HACK: node doesn't expose internal errors,
+        // we need to fake response to throw underlying errors type
+        var fakeRes = new ServerResponse({})
+        fakeRes.on('error', function (err) {
+          res.emit('error', err)
+        })
+        fakeRes.destroyed = res.destroyed
+        fakeRes.finished = res.finished || ended
+        // throw ERR_STREAM_DESTROYED or ERR_STREAM_WRITE_AFTER_END
+        _write.call(fakeRes, chunk, encoding, callback)
         return false
       }
 
       if (!res.headersSent) {
         this.writeHead(this.statusCode)
       }
 
+      if (chunk) {
+        chunk = toBuffer(chunk, encoding)
+      }
+
       return stream
-        ? stream.write(toBuffer(chunk, encoding))
-        : _write.call(this, chunk, encoding)
+        ? stream.write(chunk, encoding, callback)
+        : _write.call(this, chunk, encoding, callback)
     }
 
-    res.end = function end (chunk, encoding) {
-      if (ended) {
-        return false
+    res.end = function end (chunk, encoding, callback) {
+      if (!callback) {
+        if (typeof chunk === 'function') {
+          callback = chunk
+          chunk = encoding = undefined
+        } else if (typeof encoding === 'function') {
+          callback = encoding
+          encoding = undefined
+        }
+      }
+
+      if (this.destroyed || this.finished || ended) {
+        this.finished = ended
+        // throw ERR_STREAM_WRITE_AFTER_END or ERR_STREAM_ALREADY_FINISHED
+        return _end.call(this, chunk, encoding, callback)
       }
 
       if (!res.headersSent) {
@@ -116,16 +144,20 @@ function compression (options) {
       }
 
       if (!stream) {
-        return _end.call(this, chunk, encoding)
+        return _end.call(this, chunk, encoding, callback)
       }
 
       // mark ended
       ended = true
 
+      if (chunk) {
+        chunk = toBuffer(chunk, encoding)
+      }
+
       // write Buffer for Node.js 0.8
       return chunk
-        ? stream.end(toBuffer(chunk, encoding))
-        : stream.end()
+        ? stream.end(chunk, encoding, callback)
+        : stream.end(chunk, callback)
     }
 
     res.on = function on (type, listener) {
@@ -216,6 +248,10 @@ function compression (options) {
       res.removeHeader('Content-Length')
 
       // compression
+      stream.on('error', function (err) {
+        res.emit('error', err)
+      })
+
       stream.on('data', function onStreamData (chunk) {
         if (isFinished(res)) {
           debug('response finished')

test/compression.js

@@ -11,6 +11,211 @@ var http2 = require('http2')
 var compression = require('..')
 
 describe('compression()', function () {
+  describe('should work end and write with valid types (string, Buffer, Uint8Array)', function () {
+    it('res.write(string)', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.end('hello world')
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect('Content-Encoding', 'gzip')
+        .expect(200, done)
+    })
+
+    it('res.write(Buffer)', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.end(Buffer.from('hello world'))
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect('Content-Encoding', 'gzip')
+        .expect(200, done)
+    })
+
+    it('res.end(cb)', function (done) {
+      var callbackCalled = false
+
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.write(Buffer.from('hello world'))
+        res.end(function () {
+          callbackCalled = true
+        })
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect('Content-Encoding', 'gzip')
+        .expect(200, function () {
+          assert.ok(callbackCalled)
+          done()
+        })
+    })
+
+    it('res.end(string, cb)', function (done) {
+      var callbackCalled = false
+
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.end(Buffer.from('hello world'), function () {
+          callbackCalled = true
+        })
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect('Content-Encoding', 'gzip')
+        .expect(200, function () {
+          assert.ok(callbackCalled)
+          done()
+        })
+    })
+
+    it('res.write(Uint8Array)', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        res.end(new Uint8Array(1))
+      })
+
+      // TODO: see body response
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect('Content-Encoding', 'gzip')
+        .expect(200, done)
+    })
+  })
+
+  describe('should throw with invalid types', function () {
+    it('res.write(1) should fire ERR_INVALID_ARG_TYPE', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        try {
+          res.write(1)
+        } catch (err) {
+          assert.ok(err.code === 'ERR_INVALID_ARG_TYPE')
+          res.flush()
+          res.end()
+        }
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect(200, done)
+    })
+
+    it('res.write({}) should fire ERR_INVALID_ARG_TYPE', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        try {
+          res.write({})
+        } catch (err) {
+          assert.ok(err.code === 'ERR_INVALID_ARG_TYPE')
+          res.flush()
+          res.end()
+        }
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect(200, done)
+    })
+
+    it('res.write(null) should fire ERR_STREAM_NULL_VALUES', function (done) {
+      var server = createServer({ threshold: 0 }, function (req, res) {
+        res.setHeader('Content-Type', 'text/plain')
+        try {
+          res.write(null)
+        } catch (err) {
+          assert.ok(err.code === 'ERR_STREAM_NULL_VALUES')
+          res.flush()
+          res.end()
+        }
+      })
+
+      request(server)
+        .get('/')
+        .set('Accept-Encoding', 'gzip')
+        .expect(200, done)
+    })
+  })
+
+  it('res.write() should return false or throw ERR_STREAM_ALREADY_FINISHED when stream is already finished', function (done) {
+    var onError = function (err) {
+      assert.ok(err.toString().indexOf('write after end') > -1 || err.code === 'ERR_STREAM_WRITE_AFTER_END')
+    }
+    var server = createServer({ threshold: 0 }, function (req, res) {
+      res.on('error', onError)
+      res.setHeader('Content-Type', 'text/plain')
+      res.end('hello world')
+
+      var canWrite = res.write('hola', function (err) {
+        assert.ok(err.toString().indexOf('write after end') > -1 || err.code === 'ERR_STREAM_ALREADY_FINISHED')
+      })
+
+      assert.ok(!canWrite)
+    })
+
+    request(server)
+      .get('/')
+      .set('Accept-Encoding', 'gzip')
+      .expect(shouldHaveHeader('Content-Encoding'))
+      .expect(shouldHaveBodyLength('hello world'.length))
+      .expect(200, done)
+  })
+
+  it('res.write() should call callback if passsed', function (done) {
+    var server = createServer({ threshold: 0 }, function (req, res) {
+      res.setHeader('Content-Type', 'text/plain')
+
+      res.write('hello, world', function () {
+        res.end()
+      })
+    })
+
+    request(server)
+      .get('/')
+      .set('Accept-Encoding', 'gzip')
+      .expect(shouldHaveHeader('Content-Encoding'))
+      .expect(shouldHaveBodyLength('hello, world'.length))
+      .expect(200, done)
+  })
+
+  it('res.write() should call callback with error after end', function (done) {
+    var onErrorCalled = false
+    var onError = function (err) {
+      assert.ok(err.code === 'ERR_STREAM_WRITE_AFTER_END')
+      onErrorCalled = true
+    }
+
+    var server = createServer({ threshold: 0 }, function (req, res) {
+      res.on('error', onError)
+      res.setHeader('Content-Type', 'text/plain')
+      res.end('hello, world')
+
+      res.write('hello, world', onError)
+
+      process.nextTick(function () {
+        assert.ok(onErrorCalled)
+      })
+    })
+
+    request(server)
+      .get('/')
+      .set('Accept-Encoding', 'gzip')
+      .end(done)
+  })
+
   it('should skip HEAD', function (done) {
     var server = createServer({ threshold: 0 }, function (req, res) {
       res.setHeader('Content-Type', 'text/plain')
@@ -417,9 +622,9 @@ describe('compression()', function () {
         .expect('Content-Encoding', 'gzip', done)
     })
 
+    // TODO: why no set Content-Length?
     // res.end(str, encoding) broken in node.js 0.8
-    var run = /^v0\.8\./.test(process.version) ? it.skip : it
-    run('should handle writing hex data', function (done) {
+    it('should handle writing hex data', function (done) {
       var server = createServer({ threshold: 6 }, function (req, res) {
         res.setHeader('Content-Type', 'text/plain')
         res.end('2e2e2e2e', 'hex')
@@ -473,17 +678,30 @@ describe('compression()', function () {
     })
 
     it('should return false writing after end', function (done) {
+      var onErrorCalled = false
+      var onError = function (err) {
+        assert.ok(err.toString().indexOf('write after end') > -1 || err.code === 'ERR_STREAM_WRITE_AFTER_END')
+        onErrorCalled = true
+      }
+
       var server = createServer({ threshold: 0 }, function (req, res) {
+        res.on('error', onError)
         res.setHeader('Content-Type', 'text/plain')
+
         res.end('hello, world')
-        assert.ok(res.write() === false)
-        assert.ok(res.end() === false)
+
+        assert.ok(res.write('', onError) === false)
+
+        process.nextTick(function () {
+          assert.ok(onErrorCalled)
+        })
       })
 
       request(server)
         .get('/')
         .set('Accept-Encoding', 'gzip')
-        .expect('Content-Encoding', 'gzip', done)
+        .expect('Content-Encoding', 'gzip')
+        .end(done)
     })
   })
 
@@ -1259,9 +1477,12 @@ describe('compression()', function () {
   })
 })
 
-function createServer (opts, fn) {
+function createServer (opts, fn, t) {
   var _compression = compression(opts)
   return http.createServer(function (req, res) {
+    if (t) {
+      res.on('finish', function () { console.log(t.title, 'server closed') })
+    }
     _compression(req, res, function (err) {
       if (err) {
         res.statusCode = err.status || 500
@@ -1321,6 +1542,13 @@ function shouldHaveBodyLength (length) {
   }
 }
 
+function shouldHaveHeader (header) {
+  return function (res) {
+    var ok = (header.toLowerCase() in res.headers)
+    assert.ok(ok, 'should have header ' + header)
+  }
+}
+
 function shouldNotHaveHeader (header) {
   return function (res) {
     assert.ok(!(header.toLowerCase() in res.headers), 'should not have header ' + header)