From 5ef5116311f5e78333dc0dbe378553e64f06cab3 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sun, 14 Jun 2020 14:50:28 +0200
Subject: [PATCH] worker: rename error code to be more accurate
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rename `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
to `ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST` in order to be more
accurate.

PR-URL: https://github.com/nodejs/node/pull/33872
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 doc/api/errors.md                             | 29 ++++++++++++++++---
 src/node_errors.h                             |  4 +--
 src/node_messaging.cc                         |  4 +--
 ...worker-message-port-transfer-filehandle.js |  3 +-
 .../test-worker-workerdata-messageport.js     |  2 +-
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/doc/api/errors.md b/doc/api/errors.md
index 6bc57d656a5a97..268e66cffbcaa4 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -1597,12 +1597,20 @@ strict compliance with the API specification (which in some cases may accept
 For APIs that accept options objects, some options might be mandatory. This code
 is thrown if a required option is missing.
 
-<a id="ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST"></a>
-### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
+<a id="ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST"></a>
+### `ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`
+<!-- YAML
+added: REPLACEME
+-->
 
 An object that needs to be explicitly listed in the `transferList` argument
-was found in the object passed to a `postMessage()` call, but not provided in
-the `transferList` for that call. Usually, this is a `MessagePort`.
+was found in the object passed to a [`postMessage()`][] call, but not provided
+in the `transferList` for that call. Usually, this is a `MessagePort`.
+
+In Node.js versions prior to REPLACEME, the error code being used here was
+[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`][]. However, the set of
+transferable object types has been expanded to cover more types than
+`MessagePort`.
 
 <a id="ERR_MISSING_PASSPHRASE"></a>
 ### `ERR_MISSING_PASSPHRASE`
@@ -2442,6 +2450,16 @@ Used when an invalid character is found in an HTTP response status message
 -->
 A given index was out of the accepted range (e.g. negative offsets).
 
+<a id="ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST"></a>
+### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
+<!-- YAML
+removed: REPLACEME
+-->
+
+This error code was replaced by [`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`][]
+in Node.js REPLACEME, because it is no longer accurate as other types of
+transferable objects also exist now.
+
 <a id="ERR_NAPI_CONS_PROTOTYPE_OBJECT"></a>
 ### `ERR_NAPI_CONS_PROTOTYPE_OBJECT`
 <!-- YAML
@@ -2693,6 +2711,8 @@ such as `process.stdout.on('data')`.
 [`--force-fips`]: cli.html#cli_force_fips
 [`Class: assert.AssertionError`]: assert.html#assert_class_assert_assertionerror
 [`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE
+[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST
+[`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST
 [`EventEmitter`]: events.html#events_class_eventemitter
 [`MessagePort`]: worker_threads.html#worker_threads_class_messageport
 [`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf
@@ -2725,6 +2745,7 @@ such as `process.stdout.on('data')`.
 [`net`]: net.html
 [`new URL(input)`]: url.html#url_new_url_input_base
 [`new URLSearchParams(iterable)`]: url.html#url_new_urlsearchparams_iterable
+[`postMessage()`]: worker_threads.html#worker_threads_port_postmessage_value_transferlist
 [`process.on('exit')`]: process.html#Event:-`'exit'`
 [`process.send()`]: process.html#process_process_send_message_sendhandle_options_callback
 [`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn
diff --git a/src/node_errors.h b/src/node_errors.h
index 871071aaa07c0a..f1fa16fa276194 100644
--- a/src/node_errors.h
+++ b/src/node_errors.h
@@ -43,7 +43,7 @@ void OnFatalError(const char* location, const char* message);
   V(ERR_MEMORY_ALLOCATION_FAILED, Error)                                       \
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error)                             \
   V(ERR_MISSING_ARGS, TypeError)                                               \
-  V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError)                      \
+  V(ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST, TypeError)                      \
   V(ERR_MISSING_PASSPHRASE, TypeError)                                         \
   V(ERR_MISSING_PLATFORM_FOR_WORKER, Error)                                    \
   V(ERR_NON_CONTEXT_AWARE_DISABLED, Error)                                     \
@@ -98,7 +98,7 @@ void OnFatalError(const char* location, const char* message);
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE,                                    \
     "A message object could not be deserialized successfully in the target "   \
     "vm.Context")                                                              \
-  V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST,                                 \
+  V(ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST,                                 \
     "Object that needs transfer was found in message but not listed "          \
     "in transferList")                                                         \
   V(ERR_MISSING_PLATFORM_FOR_WORKER,                                           \
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
index 22d2ff7257234a..da2e915d02d69d 100644
--- a/src/node_messaging.cc
+++ b/src/node_messaging.cc
@@ -354,9 +354,7 @@ class SerializerDelegate : public ValueSerializer::Delegate {
       ThrowDataCloneError(env_->clone_unsupported_type_str());
       return Nothing<bool>();
     } else if (mode == BaseObject::TransferMode::kTransferable) {
-      // TODO(addaleax): This message code is too specific. Fix that in a
-      // semver-major follow-up.
-      THROW_ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST(env_);
+      THROW_ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST(env_);
       return Nothing<bool>();
     }
 
diff --git a/test/parallel/test-worker-message-port-transfer-filehandle.js b/test/parallel/test-worker-message-port-transfer-filehandle.js
index 157acb22e8ad55..e19e8c11a5ce78 100644
--- a/test/parallel/test-worker-message-port-transfer-filehandle.js
+++ b/test/parallel/test-worker-message-port-transfer-filehandle.js
@@ -14,8 +14,7 @@ const { once } = require('events');
   assert.throws(() => {
     port1.postMessage(fh);
   }, {
-    // See the TODO about error code in node_messaging.cc.
-    code: 'ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST'
+    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST'
   });
 
   // Check that transferring FileHandle instances works.
diff --git a/test/parallel/test-worker-workerdata-messageport.js b/test/parallel/test-worker-workerdata-messageport.js
index 9bf3422337e963..29a06a3196ae86 100644
--- a/test/parallel/test-worker-workerdata-messageport.js
+++ b/test/parallel/test-worker-workerdata-messageport.js
@@ -54,7 +54,7 @@ const meowScript = () => 'meow';
     workerData,
     transferList: []
   }), {
-    code: 'ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST',
+    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST',
     message: 'Object that needs transfer was found in message but not ' +
              'listed in transferList'
   });