FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

worker: emit `'messagerror'` events for failed deserialization · nodejs/node@4a37180 · GitHub

/ node Public

Commit 4a37180

Browse files
addaleax
authored and
codebytere
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 5b1fd10 commit 4a37180

9 files changed

Lines changed: 69 additions & 6 deletions

‎doc/api/errors.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,6 +1561,17 @@ behavior. See the documentation for [policy][] manifests for more information.
15611561
An attempt was made to allocate memory (usually in the C++ layer) but it
15621562
failed.
15631563

1564+
<a id="ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE"></a>
1565+
### `ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE`
1566+
<!-- YAML
1567+
added: REPLACEME
1568+
-->
1569+
1570+
A message posted to a [`MessagePort`][] could not be deserialized in the target
1571+
[vm][] `Context`. Not all Node.js objects can be successfully instantiated in
1572+
any context at this time, and attempting to transfer them using `postMessage()`
1573+
can fail on the receiving side in that case.
1574+
15641575
<a id="ERR_METHOD_NOT_IMPLEMENTED"></a>
15651576
### `ERR_METHOD_NOT_IMPLEMENTED`
15661577

@@ -2557,6 +2568,7 @@ such as `process.stdout.on('data')`.
25572568
[`Class: assert.AssertionError`]: assert.html#assert_class_assert_assertionerror
25582569
[`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE
25592570
[`EventEmitter`]: events.html#events_class_eventemitter
2571+
[`MessagePort`]: worker_threads.html#worker_threads_class_messageport
25602572
[`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf
25612573
[`Object.setPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf
25622574
[`REPL`]: repl.html

‎doc/api/worker_threads.md‎

Lines changed: 18 additions & 0 deletions
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
@@ -677,6 +686,15 @@ See the [`port.on('message')`][] event for more details.
677686
All messages sent from the worker thread will be emitted before the
678687
[`'exit'` event][] is emitted on the `Worker` object.
679688

689+
### Event: `'messageerror'`
690+
<!-- YAML
691+
added: REPLACEME
692+
-->
693+
694+
* `error` {Error} An Error object
695+
696+
The `'messageerror'` event is emitted when deserializing a message failed.
697+
680698
### Event: `'online'`
681699
<!-- YAML
682700
added: v10.5.0

‎lib/internal/worker.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ class Worker extends EventEmitter {
190190
transferList.push(...options.transferList);
191191

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

‎src/env.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ constexpr size_t kFsStatsBufferLength =
223223
V(done_string, "done") \
224224
V(duration_string, "duration") \
225225
V(ecdh_string, "ECDH") \
226+
V(emit_string, "emit") \
226227
V(emit_warning_string, "emitWarning") \
227228
V(empty_object_string, "{}") \
228229
V(encoding_string, "encoding") \
@@ -279,6 +280,7 @@ constexpr size_t kFsStatsBufferLength =
279280
V(message_port_constructor_string, "MessagePort") \
280281
V(message_port_string, "messagePort") \
281282
V(message_string, "message") \
283+
V(messageerror_string, "messageerror") \
282284
V(minttl_string, "minttl") \
283285
V(module_string, "module") \
284286
V(modulus_string, "modulus") \

‎src/node_errors.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ void OnFatalError(const char* location, const char* message);
4141
V(ERR_INVALID_ARG_TYPE, TypeError) \
4242
V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \
4343
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
44+
V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error) \
4445
V(ERR_MISSING_ARGS, TypeError) \
4546
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \
4647
V(ERR_MISSING_PASSPHRASE, TypeError) \
@@ -92,6 +93,9 @@ void OnFatalError(const char* location, const char* message);
9293
V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList") \
9394
V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \
9495
V(ERR_OSSL_EVP_INVALID_DIGEST, "Invalid digest used") \
96+
V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, \
97+
"A message object could not be deserialized successfully in the target " \
98+
"vm.Context") \
9599
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, \
96100
"Object that needs transfer was found in message but not listed " \
97101
"in transferList") \

‎src/node_messaging.cc‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,17 @@ void MessagePort::OnMessage() {
742742
Local<Function> emit_message = PersistentToLocal::Strong(emit_message_fn_);
743743

744744
Local<Value> payload;
745-
if (!ReceiveMessage(context, true).ToLocal(&payload)) goto reschedule;
745+
Local<Value> message_error;
746+
{
747+
// Catch any exceptions from parsing the message itself (not from
748+
// emitting it) as 'messageeror' events.
749+
TryCatchScope try_catch(env());
750+
if (!ReceiveMessage(context, true).ToLocal(&payload)) {
751+
if (try_catch.HasCaught() && !try_catch.HasTerminated())
752+
message_error = try_catch.Exception();
753+
goto reschedule;
754+
}
755+
}
746756
if (payload == env()->no_message_symbol()) break;
747757

748758
if (!env()->can_call_into_js()) {
@@ -753,6 +763,16 @@ void MessagePort::OnMessage() {
753763

754764
if (MakeCallback(emit_message, 1, &payload).IsEmpty()) {
755765
reschedule:
766+
if (!message_error.IsEmpty()) {
767+
// This should become a `messageerror` event in the sense of the
768+
// EventTarget API at some point.
769+
Local<Value> argv[] = {
770+
env()->messageerror_string(),
771+
message_error
772+
};
773+
USE(MakeCallback(env()->emit_string(), arraysize(argv), argv));
774+
}
775+
756776
// Re-schedule OnMessage() execution in case of failure.
757777
if (data_)
758778
TriggerAsync();
@@ -1215,8 +1235,7 @@ BaseObjectPtr<BaseObject> JSTransferable::Data::Deserialize(
12151235
// the end of the stream, after the main message has been read.
12161236

12171237
if (context != env->context()) {
1218-
// It would be nice to throw some kind of exception here, but how do we
1219-
// pass that to end users? For now, just drop the message silently.
1238+
THROW_ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE(env);
12201239
return {};
12211240
}
12221241
HandleScope handle_scope(env->isolate());

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

Lines 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();

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

Lines 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();

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

Lines 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)
Back | FazBrowse Home | New Git URL