From 3d6d1bbb99a53e0847f3f6be13730c6ba026673d Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sun, 16 Feb 2020 10:47:13 +0100
Subject: [PATCH 1/9] http: cleanup end argument handling

---
 lib/_http_outgoing.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index a7bd6af60073a7..27c26ee9d50c77 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -752,6 +752,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
   if (typeof chunk === 'function') {
     callback = chunk;
     chunk = null;
+    encoding = null;
   } else if (typeof encoding === 'function') {
     callback = encoding;
     encoding = null;
@@ -761,7 +762,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
     this.socket.cork();
   }
 
-  if (chunk) {
+  if (chunk !== null && chunk !== undefined) {
     if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
       throw new ERR_INVALID_ARG_TYPE('chunk', ['string', 'Buffer'], chunk);
     }

From 70b2b52e78360a3bc08fbe68193ae7e37d468bd2 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sun, 16 Feb 2020 11:32:42 +0100
Subject: [PATCH 2/9] http: make more Writable-like

---
 lib/_http_outgoing.js                         |  80 +++++++------
 test/parallel/test-http-outgoing-proto.js     |  38 +++---
 .../parallel/test-http-res-write-after-end.js |   4 +-
 ...test-http-res-write-end-dont-take-array.js | 108 ++++++++++++------
 4 files changed, 132 insertions(+), 98 deletions(-)

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 27c26ee9d50c77..ecfee40efba8a8 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -56,7 +56,9 @@ const {
     ERR_METHOD_NOT_IMPLEMENTED,
     ERR_STREAM_CANNOT_PIPE,
     ERR_STREAM_ALREADY_FINISHED,
-    ERR_STREAM_WRITE_AFTER_END
+    ERR_STREAM_WRITE_AFTER_END,
+    ERR_STREAM_NULL_VALUES,
+    ERR_STREAM_DESTROYED
   },
   hideStackFrames
 } = require('internal/errors');
@@ -641,30 +643,58 @@ ObjectDefineProperty(OutgoingMessage.prototype, 'writableEnded', {
 
 const crlf_buf = Buffer.from('\r\n');
 OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
+  if (typeof encoding === 'function') {
+    callback = encoding;
+    encoding = null;
+  }
+
   const ret = write_(this, chunk, encoding, callback, false);
   if (!ret)
     this[kNeedDrain] = true;
   return ret;
 };
 
-function writeAfterEnd(msg, callback) {
-  const err = new ERR_STREAM_WRITE_AFTER_END();
+function errorOrDestroy(msg, err) {
   const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined;
   defaultTriggerAsyncIdScope(triggerAsyncId,
                              process.nextTick,
-                             writeAfterEndNT,
+                             emitErrorNt,
                              msg,
-                             err,
-                             callback);
+                             err);
+}
+
+function emitErrorNt(msg, err) {
+  if (typeof msg.emit === 'function') msg.emit('error', err);
 }
 
 function write_(msg, chunk, encoding, callback, fromEnd) {
+  let err;
+  let len;
   if (msg.finished) {
-    writeAfterEnd(msg, callback);
-    return true;
+    err = new ERR_STREAM_WRITE_AFTER_END();
+  } else if (msg.destroyed) {
+    err = new ERR_STREAM_DESTROYED('write');
+  } else if (chunk === null) {
+    err = new ERR_STREAM_NULL_VALUES();
+  } else if (typeof chunk === 'string') {
+    len = Buffer.byteLength(chunk, encoding);
+  } else if (chunk instanceof Buffer) {
+    len = chunk.length;
+  } else {
+    err = new ERR_INVALID_ARG_TYPE(
+      'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
+  }
+
+  if (err) {
+    if (callback) process.nextTick(callback, err);
+    errorOrDestroy(msg, err);
+    return false;
   }
 
   if (!msg._header) {
+    if (fromEnd && chunk) {
+      msg._contentLength = len;
+    }
     msg._implicitHeader();
   }
 
@@ -675,11 +705,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
     return true;
   }
 
-  if (!fromEnd && typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
-    throw new ERR_INVALID_ARG_TYPE('first argument',
-                                   ['string', 'Buffer'], chunk);
-  }
-
   if (!fromEnd && msg.socket && !msg.socket.writableCorked) {
     msg.socket.cork();
     process.nextTick(connectionCorkNT, msg.socket);
@@ -687,12 +712,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
 
   let ret;
   if (msg.chunkedEncoding && chunk.length !== 0) {
-    let len;
-    if (typeof chunk === 'string')
-      len = Buffer.byteLength(chunk, encoding);
-    else
-      len = chunk.length;
-
     msg._send(len.toString(16), 'latin1', null);
     msg._send(crlf_buf, null, null);
     msg._send(chunk, encoding, null);
@@ -706,12 +725,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
 }
 
 
-function writeAfterEndNT(msg, err, callback) {
-  msg.emit('error', err);
-  if (callback) callback(err);
-}
-
-
 function connectionCorkNT(conn) {
   conn.uncork();
 }
@@ -762,22 +775,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
     this.socket.cork();
   }
 
-  if (chunk !== null && chunk !== undefined) {
-    if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
-      throw new ERR_INVALID_ARG_TYPE('chunk', ['string', 'Buffer'], chunk);
-    }
-
-    if (this.finished) {
-      writeAfterEnd(this, callback);
-      return this;
-    }
-
-    if (!this._header) {
-      if (typeof chunk === 'string')
-        this._contentLength = Buffer.byteLength(chunk, encoding);
-      else
-        this._contentLength = chunk.length;
-    }
+  if (chunk) {
     write_(this, chunk, encoding, null, true);
   } else if (this.finished) {
     if (typeof callback === 'function') {
diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js
index 4a07d18c601c5c..7e539e73496ab4 100644
--- a/test/parallel/test-http-outgoing-proto.js
+++ b/test/parallel/test-http-outgoing-proto.js
@@ -74,27 +74,31 @@ assert.throws(() => {
   );
 }
 
-assert(OutgoingMessage.prototype.write.call({ _header: 'test' }));
+assert(!OutgoingMessage.prototype.write.call({ _header: 'test' }));
 
-assert.throws(() => {
+{
+  const expectedError = {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "chunk" argument must be of type string or an instance of ' +
+            'Buffer or Uint8Array. Received undefined'
+  };
   const outgoingMessage = new OutgoingMessage();
-  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' });
-}, {
-  code: 'ERR_INVALID_ARG_TYPE',
-  name: 'TypeError',
-  message: 'The first argument must be of type string or an instance of ' +
-           'Buffer. Received undefined'
-});
+  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, undefined,
+                             common.expectsError(expectedError));
+}
 
-assert.throws(() => {
+{
+  const expectedError = {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+    message: 'The "chunk" argument must be of type string or an instance of ' +
+            'Buffer or Uint8Array. Received type number (1)'
+  };
   const outgoingMessage = new OutgoingMessage();
-  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, 1);
-}, {
-  code: 'ERR_INVALID_ARG_TYPE',
-  name: 'TypeError',
-  message: 'The first argument must be of type string or an instance of ' +
-           'Buffer. Received type number (1)'
-});
+  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, 1,
+                             common.expectsError(expectedError));
+}
 
 // addTrailers()
 // The `Error` comes from the JavaScript engine so confirm that it is a
diff --git a/test/parallel/test-http-res-write-after-end.js b/test/parallel/test-http-res-write-after-end.js
index 70325975c4da0f..285938c8fe4d8c 100644
--- a/test/parallel/test-http-res-write-after-end.js
+++ b/test/parallel/test-http-res-write-after-end.js
@@ -34,8 +34,8 @@ const server = http.Server(common.mustCall(function(req, res) {
   res.end();
 
   const r = res.write('This should raise an error.');
-  // Write after end should return true
-  assert.strictEqual(r, true);
+  // Write after end should return false
+  assert.strictEqual(r, false);
 }));
 
 server.listen(0, function() {
diff --git a/test/parallel/test-http-res-write-end-dont-take-array.js b/test/parallel/test-http-res-write-end-dont-take-array.js
index 8bebfc14e4bc2d..976ed3407f71d8 100644
--- a/test/parallel/test-http-res-write-end-dont-take-array.js
+++ b/test/parallel/test-http-res-write-end-dont-take-array.js
@@ -21,53 +21,85 @@
 
 'use strict';
 const common = require('../common');
-const assert = require('assert');
 const http = require('http');
 
-const server = http.createServer();
+{
+  const server = http.createServer();
 
-server.once('request', common.mustCall((req, res) => {
-  server.on('request', common.mustCall((req, res) => {
-    res.end(Buffer.from('asdf'));
+  server.once('request', common.mustCall((req, res) => {
+    // `res.write()` should accept `string`.
+    res.write('string');
+    // `res.write()` should accept `buffer`.
+    res.write(Buffer.from('asdf'));
+
+    const expectedError = {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+    };
+
+    // `res.write()` should not accept an Array.
+    res.write(['array'], common.expectsError(expectedError));
+    res.on('error', common.expectsError(expectedError));
+    res.on('error', () => {
+      res.destroy();
+      server.close();
+    });
   }));
-  // `res.write()` should accept `string`.
-  res.write('string');
-  // `res.write()` should accept `buffer`.
-  res.write(Buffer.from('asdf'));
 
-  const expectedError = {
-    code: 'ERR_INVALID_ARG_TYPE',
-    name: 'TypeError',
-  };
+  server.listen(0, function() {
+    http.get({ port: this.address().port }, (res) => {
+      res.resume();
+    });
+  });
+}
 
-  // `res.write()` should not accept an Array.
-  assert.throws(
-    () => {
-      res.write(['array']);
-    },
-    expectedError
-  );
+// TODO(ronag): end must handle onFinished w/ error better
+// {
+//   const server = http.createServer();
 
-  // `res.end()` should not accept an Array.
-  assert.throws(
-    () => {
-      res.end(['moo']);
-    },
-    expectedError
-  );
+//   server.once('request', common.mustCall((req, res) => {
+//     // `res.write()` should accept `string`.
+//     res.write('string');
+//     // `res.write()` should accept `buffer`.
+//     res.write(Buffer.from('asdf'));
 
-  // `res.end()` should accept `string`.
-  res.end('string');
-}));
+//     const expectedError = {
+//       code: 'ERR_INVALID_ARG_TYPE',
+//       name: 'TypeError',
+//     };
 
-server.listen(0, function() {
-  // Just make a request, other tests handle responses.
-  http.get({ port: this.address().port }, (res) => {
-    res.resume();
-    // Do it again to test .end(Buffer);
-    http.get({ port: server.address().port }, (res) => {
-      res.resume();
+//     // `res.end()` should not accept an Array.
+//     res.end(['array'], common.expectsError(expectedError));
+//     res.on('error', common.expectsError(expectedError));
+//     res.on('error', () => {
+//       res.destroy();
+//       server.close();
+//     });
+//   }));
+
+//   server.listen(0, function() {
+//     // Just make a request, other tests handle responses.
+//     http.get({ port: this.address().port }, (res) => {
+//       res.resume();
+//     });
+//   });
+// }
+
+{
+  const server = http.createServer();
+
+  server.once('request', common.mustCall((req, res) => {
+    // `res.end()` should accept `Buffer`.
+    res.end(Buffer.from('asdf'), () => {
+      res.destroy();
       server.close();
     });
+  }));
+
+  server.listen(0, function() {
+    // Just make a request, other tests handle responses.
+    http.get({ port: this.address().port }, (res) => {
+      res.resume();
+    });
   });
-});
+}

From 9d7403902e60833d851246ca547b5e5a1ce53950 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sun, 16 Feb 2020 21:13:18 +0100
Subject: [PATCH 3/9] fixup: test

---
 lib/_http_outgoing.js                       |  4 +-
 test/parallel/test-http-outgoing-destroy.js | 36 ++++++++++++++
 test/parallel/test-http-outgoing-finish.js  |  8 +--
 test/parallel/test-http-outgoing-proto.js   | 10 ++++
 test/parallel/test-http-outgoing-write.js   | 55 +++++++++++++++++++++
 5 files changed, 107 insertions(+), 6 deletions(-)
 create mode 100644 test/parallel/test-http-outgoing-destroy.js
 create mode 100644 test/parallel/test-http-outgoing-write.js

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index ecfee40efba8a8..8bc36d6493ece5 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -654,7 +654,7 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
   return ret;
 };
 
-function errorOrDestroy(msg, err) {
+function doDestroy(msg, err) {
   const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined;
   defaultTriggerAsyncIdScope(triggerAsyncId,
                              process.nextTick,
@@ -687,7 +687,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
 
   if (err) {
     if (callback) process.nextTick(callback, err);
-    errorOrDestroy(msg, err);
+    doDestroy(msg, err);
     return false;
   }
 
diff --git a/test/parallel/test-http-outgoing-destroy.js b/test/parallel/test-http-outgoing-destroy.js
new file mode 100644
index 00000000000000..a3e6bc3eb453ab
--- /dev/null
+++ b/test/parallel/test-http-outgoing-destroy.js
@@ -0,0 +1,36 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+const http = require('http');
+const OutgoingMessage = http.OutgoingMessage;
+
+{
+  const msg = new OutgoingMessage();
+  assert.strictEqual(msg.destroyed, false);
+  msg.destroy();
+  assert.strictEqual(msg.destroyed, true);
+  msg.write('asd', common.expectsError({
+    name: 'Error',
+    code: 'ERR_STREAM_DESTROYED'
+  }));
+  msg.on('error', common.expectsError({
+    name: 'Error',
+    code: 'ERR_STREAM_DESTROYED'
+  }));
+}
+
+{
+  const msg = new OutgoingMessage();
+  assert.strictEqual(msg.destroyed, false);
+  msg.destroy();
+  assert.strictEqual(msg.destroyed, true);
+  msg.write('end', common.expectsError({
+    name: 'Error',
+    code: 'ERR_STREAM_DESTROYED'
+  }));
+  msg.on('error', common.expectsError({
+    name: 'Error',
+    code: 'ERR_STREAM_DESTROYED'
+  }));
+}
diff --git a/test/parallel/test-http-outgoing-finish.js b/test/parallel/test-http-outgoing-finish.js
index d5b94eb0f65b8f..7464dd57ea1f31 100644
--- a/test/parallel/test-http-outgoing-finish.js
+++ b/test/parallel/test-http-outgoing-finish.js
@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 const http = require('http');
@@ -49,7 +49,7 @@ function write(out) {
   let endCb = false;
 
   // First, write until it gets some backpressure
-  while (out.write(buf)) {}
+  while (out.write(buf, common.mustCall())) {}
 
   // Now end, and make sure that we don't get the 'finish' event
   // before the tick where the cb gets called.  We give it until
@@ -65,12 +65,12 @@ function write(out) {
     });
   });
 
-  out.end(buf, function() {
+  out.end(buf, common.mustCall(function() {
     endCb = true;
     console.error(`${name} endCb`);
     process.nextTick(function() {
       assert(finishEvent, `${name} got endCb event before finishEvent!`);
       console.log(`ok - ${name} endCb`);
     });
-  });
+  }));
 }
diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js
index 7e539e73496ab4..4f853cae95f0f5 100644
--- a/test/parallel/test-http-outgoing-proto.js
+++ b/test/parallel/test-http-outgoing-proto.js
@@ -100,6 +100,16 @@ assert(!OutgoingMessage.prototype.write.call({ _header: 'test' }));
                              common.expectsError(expectedError));
 }
 
+{
+  const expectedError = {
+    code: 'ERR_STREAM_NULL_VALUES',
+    name: 'TypeError',
+  };
+  const outgoingMessage = new OutgoingMessage();
+  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, null,
+                             common.expectsError(expectedError));
+}
+
 // addTrailers()
 // The `Error` comes from the JavaScript engine so confirm that it is a
 // `TypeError` but do not check the message. It will be different in different
diff --git a/test/parallel/test-http-outgoing-write.js b/test/parallel/test-http-outgoing-write.js
new file mode 100644
index 00000000000000..829b725e129fcc
--- /dev/null
+++ b/test/parallel/test-http-outgoing-write.js
@@ -0,0 +1,55 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+const http = require('http');
+
+http.createServer(function(req, res) {
+  req.resume();
+  req.on('end', function() {
+    write(res);
+  });
+  this.close();
+}).listen(0, function() {
+  const req = http.request({
+    port: this.address().port,
+    method: 'PUT'
+  });
+  write(req);
+  req.on('response', function(res) {
+    res.resume();
+  });
+});
+
+const buf = Buffer.alloc(1024 * 16, 'x');
+function write(out) {
+  const name = out.constructor.name;
+  let finishEvent = false;
+  let endCb = false;
+
+  // First, write until it gets some backpressure
+  while (out.write(buf, common.mustCall())) {}
+
+  // Now end, and make sure that we don't get the 'finish' event
+  // before the tick where the cb gets called.  We give it until
+  // nextTick because this is added as a listener before the endcb
+  // is registered.  The order is not what we're testing here, just
+  // that 'finish' isn't emitted until the stream is fully flushed.
+  out.on('finish', function() {
+    finishEvent = true;
+    console.error(`${name} finish event`);
+    process.nextTick(function() {
+      assert(endCb, `${name} got finish event before endcb!`);
+      console.log(`ok - ${name} finishEvent`);
+    });
+  });
+
+  out.end(buf, common.mustCall(function() {
+    endCb = true;
+    console.error(`${name} endCb`);
+    process.nextTick(function() {
+      assert(finishEvent, `${name} got endCb event before finishEvent!`);
+      console.log(`ok - ${name} endCb`);
+    });
+  }));
+}

From 37ee279a68fa60000f7aa78cbfdc2af3482f6bb4 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sun, 16 Feb 2020 22:20:21 +0100
Subject: [PATCH 4/9] fixup: minor cleanup

---
 lib/_http_outgoing.js | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 8bc36d6493ece5..efe952a985b8a9 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -69,6 +69,8 @@ const { CRLF, debug } = common;
 
 const kCorked = Symbol('corked');
 
+function nop() {}
+
 const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
 const RE_TE_CHUNKED = common.chunkExpression;
 
@@ -654,7 +656,7 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
   return ret;
 };
 
-function doDestroy(msg, err) {
+function onError(msg, err) {
   const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined;
   defaultTriggerAsyncIdScope(triggerAsyncId,
                              process.nextTick,
@@ -668,6 +670,9 @@ function emitErrorNt(msg, err) {
 }
 
 function write_(msg, chunk, encoding, callback, fromEnd) {
+  if (typeof callback !== 'function')
+    callback = nop;
+
   let err;
   let len;
   if (msg.finished) {
@@ -686,13 +691,13 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
   }
 
   if (err) {
-    if (callback) process.nextTick(callback, err);
-    doDestroy(msg, err);
+    process.nextTick(callback, err);
+    onError(msg, err);
     return false;
   }
 
   if (!msg._header) {
-    if (fromEnd && chunk) {
+    if (fromEnd) {
       msg._contentLength = len;
     }
     msg._implicitHeader();
@@ -701,7 +706,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
   if (!msg._hasBody) {
     debug('This type of response MUST NOT have a body. ' +
           'Ignoring write() calls.');
-    if (callback) process.nextTick(callback);
+    process.nextTick(callback);
     return true;
   }
 

From 8c3186380f1ed912fa562ce1e71f42cdd715bff4 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sun, 16 Feb 2020 22:27:24 +0100
Subject: [PATCH 5/9] fixup: invoke callback in async id scope

---
 lib/_http_outgoing.js                       | 11 ++++----
 test/parallel/test-http-outgoing-destroy.js | 28 ++++++---------------
 2 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index efe952a985b8a9..2ce8eec9eaf2d2 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -656,16 +656,18 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
   return ret;
 };
 
-function onError(msg, err) {
+function onError(msg, err, callback) {
   const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined;
   defaultTriggerAsyncIdScope(triggerAsyncId,
                              process.nextTick,
                              emitErrorNt,
                              msg,
-                             err);
+                             err,
+                             callback);
 }
 
-function emitErrorNt(msg, err) {
+function emitErrorNt(msg, err, callback) {
+  callback(err);
   if (typeof msg.emit === 'function') msg.emit('error', err);
 }
 
@@ -691,8 +693,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
   }
 
   if (err) {
-    process.nextTick(callback, err);
-    onError(msg, err);
+    onError(msg, err, callback);
     return false;
   }
 
diff --git a/test/parallel/test-http-outgoing-destroy.js b/test/parallel/test-http-outgoing-destroy.js
index a3e6bc3eb453ab..91c68aa9af3db0 100644
--- a/test/parallel/test-http-outgoing-destroy.js
+++ b/test/parallel/test-http-outgoing-destroy.js
@@ -10,27 +10,13 @@ const OutgoingMessage = http.OutgoingMessage;
   assert.strictEqual(msg.destroyed, false);
   msg.destroy();
   assert.strictEqual(msg.destroyed, true);
-  msg.write('asd', common.expectsError({
-    name: 'Error',
-    code: 'ERR_STREAM_DESTROYED'
+  let callbackCalled = false;
+  msg.write('asd', common.mustCall((err) => {
+    assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
+    callbackCalled = true;
   }));
-  msg.on('error', common.expectsError({
-    name: 'Error',
-    code: 'ERR_STREAM_DESTROYED'
-  }));
-}
-
-{
-  const msg = new OutgoingMessage();
-  assert.strictEqual(msg.destroyed, false);
-  msg.destroy();
-  assert.strictEqual(msg.destroyed, true);
-  msg.write('end', common.expectsError({
-    name: 'Error',
-    code: 'ERR_STREAM_DESTROYED'
-  }));
-  msg.on('error', common.expectsError({
-    name: 'Error',
-    code: 'ERR_STREAM_DESTROYED'
+  msg.on('error', common.mustCall((err) => {
+    assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
+    assert.strictEqual(callbackCalled, true);
   }));
 }

From 9018a771ea848bdb45b632a4f69e67619eea80b4 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Mon, 17 Feb 2020 17:01:51 +0100
Subject: [PATCH 6/9] fixup: tests

---
 test/parallel/test-http-outgoing-write.js     | 55 -------------------
 ...test-http-res-write-end-dont-take-array.js |  2 +-
 2 files changed, 1 insertion(+), 56 deletions(-)
 delete mode 100644 test/parallel/test-http-outgoing-write.js

diff --git a/test/parallel/test-http-outgoing-write.js b/test/parallel/test-http-outgoing-write.js
deleted file mode 100644
index 829b725e129fcc..00000000000000
--- a/test/parallel/test-http-outgoing-write.js
+++ /dev/null
@@ -1,55 +0,0 @@
-'use strict';
-const common = require('../common');
-const assert = require('assert');
-
-const http = require('http');
-
-http.createServer(function(req, res) {
-  req.resume();
-  req.on('end', function() {
-    write(res);
-  });
-  this.close();
-}).listen(0, function() {
-  const req = http.request({
-    port: this.address().port,
-    method: 'PUT'
-  });
-  write(req);
-  req.on('response', function(res) {
-    res.resume();
-  });
-});
-
-const buf = Buffer.alloc(1024 * 16, 'x');
-function write(out) {
-  const name = out.constructor.name;
-  let finishEvent = false;
-  let endCb = false;
-
-  // First, write until it gets some backpressure
-  while (out.write(buf, common.mustCall())) {}
-
-  // Now end, and make sure that we don't get the 'finish' event
-  // before the tick where the cb gets called.  We give it until
-  // nextTick because this is added as a listener before the endcb
-  // is registered.  The order is not what we're testing here, just
-  // that 'finish' isn't emitted until the stream is fully flushed.
-  out.on('finish', function() {
-    finishEvent = true;
-    console.error(`${name} finish event`);
-    process.nextTick(function() {
-      assert(endCb, `${name} got finish event before endcb!`);
-      console.log(`ok - ${name} finishEvent`);
-    });
-  });
-
-  out.end(buf, common.mustCall(function() {
-    endCb = true;
-    console.error(`${name} endCb`);
-    process.nextTick(function() {
-      assert(finishEvent, `${name} got endCb event before finishEvent!`);
-      console.log(`ok - ${name} endCb`);
-    });
-  }));
-}
diff --git a/test/parallel/test-http-res-write-end-dont-take-array.js b/test/parallel/test-http-res-write-end-dont-take-array.js
index 976ed3407f71d8..58c484d9f2e27e 100644
--- a/test/parallel/test-http-res-write-end-dont-take-array.js
+++ b/test/parallel/test-http-res-write-end-dont-take-array.js
@@ -53,7 +53,7 @@ const http = require('http');
   });
 }
 
-// TODO(ronag): end must handle onFinished w/ error better
+// TODO(ronag): end doesn't always call callback
 // {
 //   const server = http.createServer();
 

From 406e65f0f656d8b0fc598fcc026af8136621256f Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 25 Feb 2020 12:11:06 +0100
Subject: [PATCH 7/9] fixup: throw

---
 lib/_http_outgoing.js | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 2ce8eec9eaf2d2..51528de8a9cc41 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -675,23 +675,25 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
   if (typeof callback !== 'function')
     callback = nop;
 
-  let err;
   let len;
-  if (msg.finished) {
-    err = new ERR_STREAM_WRITE_AFTER_END();
-  } else if (msg.destroyed) {
-    err = new ERR_STREAM_DESTROYED('write');
-  } else if (chunk === null) {
-    err = new ERR_STREAM_NULL_VALUES();
+  if (chunk === null) {
+    throw new ERR_STREAM_NULL_VALUES();
   } else if (typeof chunk === 'string') {
     len = Buffer.byteLength(chunk, encoding);
   } else if (chunk instanceof Buffer) {
     len = chunk.length;
   } else {
-    err = new ERR_INVALID_ARG_TYPE(
+    throw new ERR_INVALID_ARG_TYPE(
       'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
   }
 
+  let err;
+  if (msg.finished) {
+    err = new ERR_STREAM_WRITE_AFTER_END();
+  } else if (msg.destroyed) {
+    err = new ERR_STREAM_DESTROYED('write');
+  }
+
   if (err) {
     onError(msg, err, callback);
     return false;

From ed2dd714b999973c76ece4012623e0c7ce013b89 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 25 Feb 2020 12:16:36 +0100
Subject: [PATCH 8/9] fixup: test

---
 test/parallel/test-http-outgoing-proto.js     |  52 ++++-----
 ...test-http-res-write-end-dont-take-array.js | 108 ++++++------------
 2 files changed, 60 insertions(+), 100 deletions(-)

diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js
index 4f853cae95f0f5..21e825908fafc6 100644
--- a/test/parallel/test-http-outgoing-proto.js
+++ b/test/parallel/test-http-outgoing-proto.js
@@ -74,41 +74,33 @@ assert.throws(() => {
   );
 }
 
-assert(!OutgoingMessage.prototype.write.call({ _header: 'test' }));
-
-{
-  const expectedError = {
-    code: 'ERR_INVALID_ARG_TYPE',
-    name: 'TypeError',
-    message: 'The "chunk" argument must be of type string or an instance of ' +
-            'Buffer or Uint8Array. Received undefined'
-  };
+assert.throws(() => {
   const outgoingMessage = new OutgoingMessage();
-  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, undefined,
-                             common.expectsError(expectedError));
-}
+  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' });
+}, {
+  code: 'ERR_INVALID_ARG_TYPE',
+  name: 'TypeError',
+  message: 'The first argument must be of type string or an instance of ' +
+           'Buffer. Received undefined'
+});
 
-{
-  const expectedError = {
-    code: 'ERR_INVALID_ARG_TYPE',
-    name: 'TypeError',
-    message: 'The "chunk" argument must be of type string or an instance of ' +
-            'Buffer or Uint8Array. Received type number (1)'
-  };
+assert.throws(() => {
   const outgoingMessage = new OutgoingMessage();
-  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, 1,
-                             common.expectsError(expectedError));
-}
+  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, 1);
+}, {
+  code: 'ERR_INVALID_ARG_TYPE',
+  name: 'TypeError',
+  message: 'The first argument must be of type string or an instance of ' +
+           'Buffer. Received type number (1)'
+});
 
-{
-  const expectedError = {
-    code: 'ERR_STREAM_NULL_VALUES',
-    name: 'TypeError',
-  };
+assert.throws(() => {
   const outgoingMessage = new OutgoingMessage();
-  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, null,
-                             common.expectsError(expectedError));
-}
+  outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, null);
+}, {
+  code: 'ERR_STREAM_NULL_VALUES',
+  name: 'TypeError'
+});
 
 // addTrailers()
 // The `Error` comes from the JavaScript engine so confirm that it is a
diff --git a/test/parallel/test-http-res-write-end-dont-take-array.js b/test/parallel/test-http-res-write-end-dont-take-array.js
index 58c484d9f2e27e..8bebfc14e4bc2d 100644
--- a/test/parallel/test-http-res-write-end-dont-take-array.js
+++ b/test/parallel/test-http-res-write-end-dont-take-array.js
@@ -21,85 +21,53 @@
 
 'use strict';
 const common = require('../common');
+const assert = require('assert');
 const http = require('http');
 
-{
-  const server = http.createServer();
+const server = http.createServer();
 
-  server.once('request', common.mustCall((req, res) => {
-    // `res.write()` should accept `string`.
-    res.write('string');
-    // `res.write()` should accept `buffer`.
-    res.write(Buffer.from('asdf'));
-
-    const expectedError = {
-      code: 'ERR_INVALID_ARG_TYPE',
-      name: 'TypeError',
-    };
-
-    // `res.write()` should not accept an Array.
-    res.write(['array'], common.expectsError(expectedError));
-    res.on('error', common.expectsError(expectedError));
-    res.on('error', () => {
-      res.destroy();
-      server.close();
-    });
+server.once('request', common.mustCall((req, res) => {
+  server.on('request', common.mustCall((req, res) => {
+    res.end(Buffer.from('asdf'));
   }));
+  // `res.write()` should accept `string`.
+  res.write('string');
+  // `res.write()` should accept `buffer`.
+  res.write(Buffer.from('asdf'));
 
-  server.listen(0, function() {
-    http.get({ port: this.address().port }, (res) => {
-      res.resume();
-    });
-  });
-}
-
-// TODO(ronag): end doesn't always call callback
-// {
-//   const server = http.createServer();
+  const expectedError = {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+  };
 
-//   server.once('request', common.mustCall((req, res) => {
-//     // `res.write()` should accept `string`.
-//     res.write('string');
-//     // `res.write()` should accept `buffer`.
-//     res.write(Buffer.from('asdf'));
+  // `res.write()` should not accept an Array.
+  assert.throws(
+    () => {
+      res.write(['array']);
+    },
+    expectedError
+  );
 
-//     const expectedError = {
-//       code: 'ERR_INVALID_ARG_TYPE',
-//       name: 'TypeError',
-//     };
+  // `res.end()` should not accept an Array.
+  assert.throws(
+    () => {
+      res.end(['moo']);
+    },
+    expectedError
+  );
 
-//     // `res.end()` should not accept an Array.
-//     res.end(['array'], common.expectsError(expectedError));
-//     res.on('error', common.expectsError(expectedError));
-//     res.on('error', () => {
-//       res.destroy();
-//       server.close();
-//     });
-//   }));
+  // `res.end()` should accept `string`.
+  res.end('string');
+}));
 
-//   server.listen(0, function() {
-//     // Just make a request, other tests handle responses.
-//     http.get({ port: this.address().port }, (res) => {
-//       res.resume();
-//     });
-//   });
-// }
-
-{
-  const server = http.createServer();
-
-  server.once('request', common.mustCall((req, res) => {
-    // `res.end()` should accept `Buffer`.
-    res.end(Buffer.from('asdf'), () => {
-      res.destroy();
-      server.close();
-    });
-  }));
-
-  server.listen(0, function() {
-    // Just make a request, other tests handle responses.
-    http.get({ port: this.address().port }, (res) => {
+server.listen(0, function() {
+  // Just make a request, other tests handle responses.
+  http.get({ port: this.address().port }, (res) => {
+    res.resume();
+    // Do it again to test .end(Buffer);
+    http.get({ port: server.address().port }, (res) => {
       res.resume();
+      server.close();
     });
   });
-}
+});

From 1b51407ce6875ab95ad3bfda95933ed43401ce81 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 25 Feb 2020 14:04:32 +0100
Subject: [PATCH 9/9] fixup: test

---
 test/parallel/test-http-outgoing-proto.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js
index 21e825908fafc6..2d265c7ba64968 100644
--- a/test/parallel/test-http-outgoing-proto.js
+++ b/test/parallel/test-http-outgoing-proto.js
@@ -80,8 +80,8 @@ assert.throws(() => {
 }, {
   code: 'ERR_INVALID_ARG_TYPE',
   name: 'TypeError',
-  message: 'The first argument must be of type string or an instance of ' +
-           'Buffer. Received undefined'
+  message: 'The "chunk" argument must be of type string or an instance of ' +
+           'Buffer or Uint8Array. Received undefined'
 });
 
 assert.throws(() => {
@@ -90,8 +90,8 @@ assert.throws(() => {
 }, {
   code: 'ERR_INVALID_ARG_TYPE',
   name: 'TypeError',
-  message: 'The first argument must be of type string or an instance of ' +
-           'Buffer. Received type number (1)'
+  message: 'The "chunk" argument must be of type string or an instance of ' +
+           'Buffer or Uint8Array. Received type number (1)'
 });
 
 assert.throws(() => {