fix(response): correct response.writable logic (#781)

lib/response.js

@@ -478,16 +478,19 @@ module.exports = {
 
   /**
    * Checks if the request is writable.
-   * Tests for the existence of the socket
-   * as node sometimes does not set it.
    *
    * @return {Boolean}
    * @api private
    */
 
   get writable() {
+    // can't write any more after response finished
+    if (this.res.finished) return false;
+
     var socket = this.res.socket;
-    if (!socket) return false;
+    // There are already pending outgoing res, but still writable
+    // https://github.com/nodejs/node/blob/v4.4.7/lib/_http_server.js#L486
+    if (!socket) return true;
     return socket.writable;
   },
 

test/application.js

@@ -6,6 +6,7 @@ var request = require('supertest');
 var statuses = require('statuses');
 var assert = require('assert');
 var http = require('http');
+var net = require('net');
 var koa = require('..');
 var fs = require('fs');
 var AssertionError = assert.AssertionError;

test/response/writeable.js

@@ -1,19 +1,105 @@
 
 'use strict';
 
-var response = require('../context').response;
+var should = require('should');
+var koa = require('../../');
+var net = require('net');
 
 describe('res.writable', function(){
-  it('should return the request is writable', function(){
-    var res = response();
-    res.writable.should.be.ok;
+  describe('when continuous requests in one persistent connection', function() {
+    function requestTwice(server, done) {
+      let port = server.address().port;
+      var buf = new Buffer('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n');
+      var client = net.connect(port);
+      var datas = [];
+      client
+        .on('error', done)
+        .on('data', function(data) {
+          datas.push(data);
+        })
+        .on('end', function() {
+          done(null, datas);
+        });
+      setImmediate(function() {
+        client.write(buf);
+      });
+      setImmediate(function() {
+        client.write(buf);
+      });
+      setTimeout(function() {
+        client.end();
+      }, 100);
+    }
+
+    it('should always writable and response all requests', function(done) {
+      var app = koa();
+      var count = 0;
+      app.use(function*() {
+        count++;
+        this.body = 'request ' + count + ', writable: ' + this.writable;
+      });
+
+      var server = app.listen();
+      requestTwice(server, function(err, datas) {
+        var responses = Buffer.concat(datas).toString();
+        responses.should.match(/request 1, writable: true/);
+        responses.should.match(/request 2, writable: true/);
+        done();
+      });
+    })
   })
 
-  describe('when res.socket not present', function (){
-    it('should return the request is not writable', function (){
-      var res = response();
-      res.res.socket = null;
-      res.writable.should.not.be.ok;
+  describe('when socket closed before response sent', function() {
+    function requsetClosed(server) {
+      let port = server.address().port;
+      var buf = new Buffer('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n');
+      var client = net.connect(port);
+      setImmediate(function() {
+        client.write(buf);
+        client.end();
+      });
+    }
+
+    it('should not writable', function(done) {
+      var app = koa();
+      app.use(function*() {
+        yield sleep(1000);
+        if (this.writable) return done(new Error('this.writable should not be true'));
+        done();
+      });
+      var server = app.listen();
+      requsetClosed(server);
+    })
+  })
+
+  describe('when resposne finished', function() {
+    function request(server) {
+      let port = server.address().port;
+      var buf = new Buffer('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n');
+      var client = net.connect(port);
+      setImmediate(function() {
+        client.write(buf);
+      });
+      setTimeout(function() {
+        client.end();
+      }, 100);
+    }
+
+    it('should not writable', function(done) {
+      var app = koa();
+      app.use(function*() {
+        this.res.end();
+        if (this.writable) return done(new Error('this.writable should not be true'));
+        done();
+      });
+      var server = app.listen();
+      request(server);
     })
   })
 })
+
+function sleep(time) {
+  return new Promise(function(resolve) {
+    setTimeout(resolve, time);
+  });
+}