stream: complete migration to internal/errors

mcollina · mcollina · commit 6e86a6651ccd · 2017-10-29T18:57:28.000+01:00
Complete the migration to the new error system of _stream_readable and _stream_writable. Adds the corresponding documentation. PR-URL: #16589 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -1317,6 +1317,18 @@ Node.js does not allow `stdout` or `stderr` Streams to be closed by user code.
 Used when an attempt is made to close the `process.stdout` stream. By design,
 Node.js does not allow `stdout` or `stderr` Streams to be closed by user code.
 
+<a id="ERR_STREAM_CANNOT_PIPE"></a>
+### ERR_STREAM_CANNOT_PIPE
+
+Used when an attempt is made to call [`stream.pipe()`][] on a
+[`Writable`][] stream.
+
+<a id="ERR_STREAM_NULL_VALUES"></a>
+### ERR_STREAM_NULL_VALUES
+
+Used when an attempt is made to call [`stream.write()`][] with a `null`
+chunk.
+
 <a id="ERR_STREAM_PUSH_AFTER_EOF"></a>
 ### ERR_STREAM_PUSH_AFTER_EOF
 
@@ -1349,6 +1361,12 @@ const instance = new Socket();
 instance.setEncoding('utf8');
 ```
 
+<a id="ERR_STREAM_WRITE_AFTER_END"></a>
+### ERR_STREAM_WRITE_AFTER_END
+
+Used when an attempt is made to call [`stream.write()`][] after
+`stream.end()` has been called.
+
 <a id="ERR_TLS_CERT_ALTNAME_INVALID"></a>
 ### ERR_TLS_CERT_ALTNAME_INVALID
 
@@ -1489,6 +1507,8 @@ Used when creation of a [`zlib`][] object fails due to incorrect configuration.
 [`sign.sign()`]: crypto.html#crypto_sign_sign_privatekey_outputformat
 [`stream.push()`]: stream.html#stream_readable_push_chunk_encoding
 [`stream.unshift()`]: stream.html#stream_readable_unshift_chunk
+[`stream.write()`]: stream.html#stream_writable_write_chunk_encoding_callback
+[`Writable`]: stream.html#stream_class_stream_writable
 [`subprocess.kill()`]: child_process.html#child_process_subprocess_kill_signal
 [`subprocess.send()`]: child_process.html#child_process_subprocess_send_message_sendhandle_options_callback
 [`fs.readFileSync`]: fs.html#fs_fs_readfilesync_path_options
diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js
@@ -284,7 +284,8 @@ function chunkInvalid(state, chunk) {
       typeof chunk !== 'string' &&
       chunk !== undefined &&
       !state.objectMode) {
-    er = new TypeError('Invalid non-string/buffer chunk');
+    er = new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                              'chunk', 'string/Buffer/Uint8Array');
   }
   return er;
 }
diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
@@ -229,12 +229,12 @@ function Writable(options) {
 
 // Otherwise people can pipe Writable streams, which is just wrong.
 Writable.prototype.pipe = function() {
-  this.emit('error', new Error('Cannot pipe, not readable'));
+  this.emit('error', new errors.Error('ERR_STREAM_CANNOT_PIPE'));
 };
 
 
 function writeAfterEnd(stream, cb) {
-  var er = new Error('write after end');
+  var er = new errors.Error('ERR_STREAM_WRITE_AFTER_END');
   // TODO: defer error events consistently everywhere, not just the cb
   stream.emit('error', er);
   process.nextTick(cb, er);
@@ -248,11 +248,11 @@ function validChunk(stream, state, chunk, cb) {
   var er = false;
 
   if (chunk === null) {
-    er = new TypeError('May not write null values to stream');
+    er = new errors.TypeError('ERR_STREAM_NULL_VALUES');
   } else if (typeof chunk !== 'string' &&
              chunk !== undefined &&
              !state.objectMode) {
-    er = new TypeError('Invalid non-string/buffer chunk');
+    er = new errors.TypeError('ERR_INVALID_ARG_TYPE', 'chunk', 'string/buffer');
   }
   if (er) {
     stream.emit('error', er);
@@ -533,7 +533,7 @@ function clearBuffer(stream, state) {
 }
 
 Writable.prototype._write = function(chunk, encoding, cb) {
-  cb(new Error('_write() is not implemented'));
+  cb(new errors.Error('ERR_METHOD_NOT_IMPLEMENTED', '_transform'));
 };
 
 Writable.prototype._writev = null;
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -325,10 +325,13 @@ E('ERR_SOCKET_CLOSED', 'Socket is closed');
 E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running');
 E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed');
 E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed');
+E('ERR_STREAM_CANNOT_PIPE', 'Cannot pipe, not readable');
+E('ERR_STREAM_NULL_VALUES', 'May not write null values to stream');
 E('ERR_STREAM_PUSH_AFTER_EOF', 'stream.push() after EOF');
 E('ERR_STREAM_READ_NOT_IMPLEMENTED', '_read() is not implemented');
 E('ERR_STREAM_UNSHIFT_AFTER_END_EVENT', 'stream.unshift() after end event');
 E('ERR_STREAM_WRAP', 'Stream has StringDecoder set or is in objectMode');
+E('ERR_STREAM_WRITE_AFTER_END', 'write after end');
 E('ERR_TLS_CERT_ALTNAME_INVALID',
   'Hostname/IP does not match certificate\'s altnames: %s');
 E('ERR_TLS_DH_PARAM_SIZE', (size) =>
diff --git a/test/parallel/test-file-write-stream.js b/test/parallel/test-file-write-stream.js
@@ -64,10 +64,17 @@ file
     assert.strictEqual(file.bytesWritten, EXPECTED.length * 2);
 
     callbacks.close++;
-    assert.throws(function() {
-      console.error('write after end should not be allowed');
-      file.write('should not work anymore');
-    }, /^Error: write after end$/);
+    common.expectsError(
+      () => {
+        console.error('write after end should not be allowed');
+        file.write('should not work anymore');
+      },
+      {
+        code: 'ERR_STREAM_WRITE_AFTER_END',
+        type: Error,
+        message: 'write after end'
+      }
+    );
 
     fs.unlinkSync(fn);
   });
diff --git a/test/parallel/test-http2-head-request.js b/test/parallel/test-http2-head-request.js
@@ -8,6 +8,7 @@ const http2 = require('http2');
 
 const errCheck = common.expectsError({
   type: Error,
+  code: 'ERR_STREAM_WRITE_AFTER_END',
   message: 'write after end'
 }, 2);
 
diff --git a/test/parallel/test-stream-readable-invalid-chunk.js b/test/parallel/test-stream-readable-invalid-chunk.js
@@ -1,14 +1,19 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const stream = require('stream');
-const assert = require('assert');
 
 const readable = new stream.Readable({
   read: () => {}
 });
 
-const errMessage = /Invalid non-string\/buffer chunk/;
-assert.throws(() => readable.push([]), errMessage);
-assert.throws(() => readable.push({}), errMessage);
-assert.throws(() => readable.push(0), errMessage);
+function checkError(fn) {
+  common.expectsError(fn, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError
+  });
+}
+
+checkError(() => readable.push([]));
+checkError(() => readable.push({}));
+checkError(() => readable.push(0));
diff --git a/test/parallel/test-stream-writable-null.js b/test/parallel/test-stream-writable-null.js
@@ -1,5 +1,5 @@
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 const stream = require('stream');
@@ -16,10 +16,18 @@ MyWritable.prototype._write = function(chunk, encoding, callback) {
   callback();
 };
 
-assert.throws(() => {
-  const m = new MyWritable({ objectMode: true });
-  m.write(null, (err) => assert.ok(err));
-}, /^TypeError: May not write null values to stream$/);
+common.expectsError(
+  () => {
+    const m = new MyWritable({ objectMode: true });
+    m.write(null, (err) => assert.ok(err));
+  },
+  {
+    code: 'ERR_STREAM_NULL_VALUES',
+    type: TypeError,
+    message: 'May not write null values to stream'
+  }
+);
+
 assert.doesNotThrow(() => {
   const m = new MyWritable({ objectMode: true }).on('error', (e) => {
     assert.ok(e);
@@ -29,10 +37,17 @@ assert.doesNotThrow(() => {
   });
 });
 
-assert.throws(() => {
-  const m = new MyWritable();
-  m.write(false, (err) => assert.ok(err));
-}, /^TypeError: Invalid non-string\/buffer chunk$/);
+common.expectsError(
+  () => {
+    const m = new MyWritable();
+    m.write(false, (err) => assert.ok(err));
+  },
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError
+  }
+);
+
 assert.doesNotThrow(() => {
   const m = new MyWritable().on('error', (e) => {
     assert.ok(e);

Original file line number	Diff line number	Diff line change
`@@ -284,7 +284,8 @@ function chunkInvalid(state, chunk) {`
`284`	`284`	`typeof chunk !== 'string' &&`
`285`	`285`	`chunk !== undefined &&`
`286`	`286`	`!state.objectMode) {`
`287`		`- er = new TypeError('Invalid non-string/buffer chunk');`
	`287`	`+ er = new errors.TypeError('ERR_INVALID_ARG_TYPE',`
	`288`	`+ 'chunk', 'string/Buffer/Uint8Array');`
`288`	`289`	`}`
`289`	`290`	`return er;`
`290`	`291`	`}`