Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit dafa380

Browse filesBrowse files
addaleaxaddaleax
committed
worker: emit 'messagerror' events for failed deserialization
This is much nicer than just treating exceptions as uncaught, and enables reporting of exceptions from the internal C++ deserialization machinery. PR-URL: #33772 Backport-PR-URL: #33965 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent dd51ba3 commit dafa380
Copy full SHA for dafa380
Expand file treeCollapse file tree

9 files changed

+69
-6
lines changed
Open diff view settings
Collapse file

‎doc/api/errors.md‎

Copy file name to clipboardExpand all lines: doc/api/errors.md
+12Lines changed: 12 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1539,6 +1539,17 @@ behavior. See the documentation for [policy][] manifests for more information.
15391539
An attempt was made to allocate memory (usually in the C++ layer) but it
15401540
failed.
15411541

1542+
<a id="ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE"></a>
1543+
### `ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE`
1544+
<!-- YAML
1545+
added: REPLACEME
1546+
-->
1547+
1548+
A message posted to a [`MessagePort`][] could not be deserialized in the target
1549+
[vm][] `Context`. Not all Node.js objects can be successfully instantiated in
1550+
any context at this time, and attempting to transfer them using `postMessage()`
1551+
can fail on the receiving side in that case.
1552+
15421553
<a id="ERR_METHOD_NOT_IMPLEMENTED"></a>
15431554
### `ERR_METHOD_NOT_IMPLEMENTED`
15441555

@@ -2534,6 +2545,7 @@ such as `process.stdout.on('data')`.
25342545
[`Class: assert.AssertionError`]: assert.html#assert_class_assert_assertionerror
25352546
[`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE
25362547
[`EventEmitter`]: events.html#events_class_eventemitter
2548+
[`MessagePort`]: worker_threads.html#worker_threads_class_messageport
25372549
[`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf
25382550
[`Object.setPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf
25392551
[`REPL`]: repl.html
Collapse file

‎doc/api/worker_threads.md‎

Copy file name to clipboardExpand all lines: doc/api/worker_threads.md
+18Lines changed: 18 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,15 @@ input of [`port.postMessage()`][].
303303
Listeners on this event will receive a clone of the `value` parameter as passed
304304
to `postMessage()` and no further arguments.
305305

306+
### Event: `'messageerror'`
307+
<!-- YAML
308+
added: REPLACEME
309+
-->
310+
311+
* `error` {Error} An Error object
312+
313+
The `'messageerror'` event is emitted when deserializing a message failed.
314+
306315
### `port.close()`
307316
<!-- YAML
308317
added: v10.5.0
@@ -683,6 +692,15 @@ See the [`port.on('message')`][] event for more details.
683692
All messages sent from the worker thread will be emitted before the
684693
[`'exit'` event][] is emitted on the `Worker` object.
685694

695+
### Event: `'messageerror'`
696+
<!-- YAML
697+
added: REPLACEME
698+
-->
699+
700+
* `error` {Error} An Error object
701+
702+
The `'messageerror'` event is emitted when deserializing a message failed.
703+
686704
### Event: `'online'`
687705
<!-- YAML
688706
added: v10.5.0
Collapse file

‎lib/internal/worker.js‎

Copy file name to clipboardExpand all lines: lib/internal/worker.js
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,9 @@ class Worker extends EventEmitter {
191191
transferList.push(...options.transferList);
192192

193193
this[kPublicPort] = port1;
194-
this[kPublicPort].on('message', (message) => this.emit('message', message));
194+
for (const event of ['message', 'messageerror']) {
195+
this[kPublicPort].on(event, (message) => this.emit(event, message));
196+
}
195197
setupPortReferencing(this[kPublicPort], this, 'message');
196198
this[kPort].postMessage({
197199
argv,
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ constexpr size_t kFsStatsBufferLength =
231231
V(done_string, "done") \
232232
V(duration_string, "duration") \
233233
V(ecdh_string, "ECDH") \
234+
V(emit_string, "emit") \
234235
V(emit_warning_string, "emitWarning") \
235236
V(empty_object_string, "{}") \
236237
V(encoding_string, "encoding") \
@@ -288,6 +289,7 @@ constexpr size_t kFsStatsBufferLength =
288289
V(message_port_constructor_string, "MessagePort") \
289290
V(message_port_string, "messagePort") \
290291
V(message_string, "message") \
292+
V(messageerror_string, "messageerror") \
291293
V(minttl_string, "minttl") \
292294
V(module_string, "module") \
293295
V(modulus_string, "modulus") \
Collapse file

‎src/node_errors.h‎

Copy file name to clipboardExpand all lines: src/node_errors.h
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ void OnFatalError(const char* location, const char* message);
4242
V(ERR_INVALID_ARG_TYPE, TypeError) \
4343
V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \
4444
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
45+
V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error) \
4546
V(ERR_MISSING_ARGS, TypeError) \
4647
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \
4748
V(ERR_MISSING_PASSPHRASE, TypeError) \
@@ -96,6 +97,9 @@ void OnFatalError(const char* location, const char* message);
9697
V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList") \
9798
V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \
9899
V(ERR_OSSL_EVP_INVALID_DIGEST, "Invalid digest used") \
100+
V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, \
101+
"A message object could not be deserialized successfully in the target " \
102+
"vm.Context") \
99103
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, \
100104
"Object that needs transfer was found in message but not listed " \
101105
"in transferList") \
Collapse file

‎src/node_messaging.cc‎

Copy file name to clipboardExpand all lines: src/node_messaging.cc
+22-3Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,17 @@ void MessagePort::OnMessage() {
772772
Local<Function> emit_message = PersistentToLocal::Strong(emit_message_fn_);
773773

774774
Local<Value> payload;
775-
if (!ReceiveMessage(context, true).ToLocal(&payload)) goto reschedule;
775+
Local<Value> message_error;
776+
{
777+
// Catch any exceptions from parsing the message itself (not from
778+
// emitting it) as 'messageeror' events.
779+
TryCatchScope try_catch(env());
780+
if (!ReceiveMessage(context, true).ToLocal(&payload)) {
781+
if (try_catch.HasCaught() && !try_catch.HasTerminated())
782+
message_error = try_catch.Exception();
783+
goto reschedule;
784+
}
785+
}
776786
if (payload == env()->no_message_symbol()) break;
777787

778788
if (!env()->can_call_into_js()) {
@@ -783,6 +793,16 @@ void MessagePort::OnMessage() {
783793

784794
if (MakeCallback(emit_message, 1, &payload).IsEmpty()) {
785795
reschedule:
796+
if (!message_error.IsEmpty()) {
797+
// This should become a `messageerror` event in the sense of the
798+
// EventTarget API at some point.
799+
Local<Value> argv[] = {
800+
env()->messageerror_string(),
801+
message_error
802+
};
803+
USE(MakeCallback(env()->emit_string(), arraysize(argv), argv));
804+
}
805+
786806
// Re-schedule OnMessage() execution in case of failure.
787807
if (data_)
788808
TriggerAsync();
@@ -1245,8 +1265,7 @@ BaseObjectPtr<BaseObject> JSTransferable::Data::Deserialize(
12451265
// the end of the stream, after the main message has been read.
12461266

12471267
if (context != env->context()) {
1248-
// It would be nice to throw some kind of exception here, but how do we
1249-
// pass that to end users? For now, just drop the message silently.
1268+
THROW_ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE(env);
12501269
return {};
12511270
}
12521271
HandleScope handle_scope(env->isolate());
Collapse file

‎test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-worker-message-port-transfer-fake-js-transferable-internal.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const { once } = require('events');
2626
port1.postMessage(fh, [ fh ]);
2727
port2.on('message', common.mustNotCall());
2828

29-
const [ exception ] = await once(process, 'uncaughtException');
29+
const [ exception ] = await once(port2, 'messageerror');
3030

3131
assert.strictEqual(exception.message, 'Unknown deserialize spec net:Socket');
3232
port2.close();
Collapse file

‎test/parallel/test-worker-message-port-transfer-fake-js-transferable.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-worker-message-port-transfer-fake-js-transferable.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ module.exports = {
3030
port1.postMessage(fh, [ fh ]);
3131
port2.on('message', common.mustNotCall());
3232

33-
const [ exception ] = await once(process, 'uncaughtException');
33+
const [ exception ] = await once(port2, 'messageerror');
3434

3535
assert.match(exception.message, /Missing internal module/);
3636
port2.close();
Collapse file

‎test/parallel/test-worker-message-port-transfer-filehandle.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-worker-message-port-transfer-filehandle.js
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ const { once } = require('events');
5555
assert.strictEqual(msgEvent.data, 'second message');
5656
port1.close();
5757
});
58+
// TODO(addaleax): Switch this to a 'messageerror' event once MessagePort
59+
// implements EventTarget fully and in a cross-context manner.
60+
port2moved.emit = common.mustCall((name, err) => {
61+
assert.strictEqual(name, 'messageerror');
62+
assert.strictEqual(err.code, 'ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE');
63+
});
5864
port2moved.start();
5965

6066
assert.notStrictEqual(fh.fd, -1);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.