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

lib/response.js

@@ -491,8 +491,13 @@ module.exports = {
    */
 
   get writable() {
+    // can't write any more after response finished
+    if (this.res.finished) return false;
+
     const 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/response/writeable.js

@@ -1,19 +1,94 @@
 
 'use strict';
 
-const response = require('../helpers/context').response;
+const Koa = require('../../');
+const net = require('net');
 
 describe('res.writable', () => {
-  it('should return the request is writable', () => {
-    const res = response();
-    res.writable.should.be.ok;
+  describe('when continuous requests in one persistent connection', () => {
+    function requestTwice(server, done){
+      const port = server.address().port;
+      const buf = new Buffer('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n');
+      const client = net.connect(port);
+      const datas = [];
+      client
+        .on('error', done)
+        .on('data', data => datas.push(data))
+        .on('end', () => done(null, datas));
+      setImmediate(() => client.write(buf));
+      setImmediate(() => client.write(buf));
+      setTimeout(() => client.end(), 100);
+    }
+
+    it('should always writable and response all requests', done => {
+      const app = new Koa();
+      let count = 0;
+      app.use(ctx => {
+        count++;
+        ctx.body = 'request ' + count + ', writable: ' + ctx.writable;
+      });
+
+      const server = app.listen();
+      requestTwice(server, (_, datas) => {
+        const 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', () => {
-    it('should return the request is not writable', () => {
-      const res = response();
-      res.res.socket = null;
-      res.writable.should.not.be.ok;
+  describe('when socket closed before response sent', () => {
+    function requsetClosed(server){
+      const port = server.address().port;
+      const buf = new Buffer('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n');
+      const client = net.connect(port);
+      setImmediate(() => {
+        client.write(buf);
+        client.end();
+      });
+    }
+
+    it('should not writable', done => {
+      const app = new Koa();
+      app.use(ctx => {
+        sleep(1000)
+        .then(() => {
+          if (ctx.writable) return done(new Error('ctx.writable should not be true'));
+          done();
+        });
+      });
+      const server = app.listen();
+      requsetClosed(server);
+    });
+  });
+
+  describe('when response finished', () => {
+    function request(server){
+      const port = server.address().port;
+      const buf = new Buffer('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n');
+      const client = net.connect(port);
+      setImmediate(() => {
+        client.write(buf);
+      });
+      setTimeout(() => {
+        client.end();
+      }, 100);
+    }
+
+    it('should not writable', done => {
+      const app = new Koa();
+      app.use(ctx => {
+        ctx.res.end();
+        if (ctx.writable) return done(new Error('ctx.writable should not be true'));
+        done();
+      });
+      const server = app.listen();
+      request(server);
     });
   });
 });
+
+function sleep(time){
+  return new Promise(resolve => setTimeout(resolve, time));
+}