lib: trigger uncaught exception handler for microtasks

PR-URL: #23794 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me>
nodejs · Oct 24, 2018 · 22cd537 · 22cd537
1 parent b07cb48
commit 22cd537
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 25 deletions.
diff --git a/doc/api/globals.md b/doc/api/globals.md
@@ -119,8 +119,8 @@ added: v11.0.0
 * `callback` {Function} Function to be queued.
 
 The `queueMicrotask()` method queues a microtask to invoke `callback`. If
-`callback` throws an exception, the [`process` object][] `'error'` event will
-be emitted.
+`callback` throws an exception, the [`process` object][] `'uncaughtException'`
+event will be emitted.
 
 In general, `queueMicrotask` is the idiomatic choice over `process.nextTick()`.
 `process.nextTick()` will always run before the microtask queue, and so

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -24,7 +24,8 @@
                                 _umask, _initgroups, _setegid, _seteuid,
                                 _setgid, _setuid, _setgroups,
                                 _shouldAbortOnUncaughtToggle },
-                              { internalBinding, NativeModule }) {
+                              { internalBinding, NativeModule },
+                              triggerFatalException) {
   const exceptionHandlerState = { captureFn: null };
   const isMainThread = internalBinding('worker').threadId === 0;
 
@@ -538,8 +539,9 @@
       get: () => {
         process.emitWarning('queueMicrotask() is experimental.',
                             'ExperimentalWarning');
-        const { queueMicrotask } =
+        const { setupQueueMicrotask } =
           NativeModule.require('internal/queue_microtask');
+        const queueMicrotask = setupQueueMicrotask(triggerFatalException);
         Object.defineProperty(global, 'queueMicrotask', {
           value: queueMicrotask,
           writable: true,

diff --git a/lib/internal/queue_microtask.js b/lib/internal/queue_microtask.js
@@ -5,27 +5,35 @@ const { AsyncResource } = require('async_hooks');
 const { getDefaultTriggerAsyncId } = require('internal/async_hooks');
 const { enqueueMicrotask } = internalBinding('util');
 
-// declared separately for name, arrow function to prevent construction
-const queueMicrotask = (callback) => {
-  if (typeof callback !== 'function') {
-    throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
-  }
+const setupQueueMicrotask = (triggerFatalException) => {
+  const queueMicrotask = (callback) => {
+    if (typeof callback !== 'function') {
+      throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
+    }
 
-  const asyncResource = new AsyncResource('Microtask', {
-    triggerAsyncId: getDefaultTriggerAsyncId(),
-    requireManualDestroy: true,
-  });
+    const asyncResource = new AsyncResource('Microtask', {
+      triggerAsyncId: getDefaultTriggerAsyncId(),
+      requireManualDestroy: true,
+    });
 
-  enqueueMicrotask(() => {
-    asyncResource.runInAsyncScope(() => {
-      try {
-        callback();
-      } catch (e) {
-        process.emit('error', e);
-      }
+    enqueueMicrotask(() => {
+      asyncResource.runInAsyncScope(() => {
+        try {
+          callback();
+        } catch (error) {
+          // TODO(devsnek) remove this if
+          // https://bugs.chromium.org/p/v8/issues/detail?id=8326
+          // is resolved such that V8 triggers the fatal exception
+          // handler for microtasks
+          triggerFatalException(error);
+        } finally {
+          asyncResource.emitDestroy();
+        }
+      });
     });
-    asyncResource.emitDestroy();
-  });
+  };
+
+  return queueMicrotask;
 };
 
-module.exports = { queueMicrotask };
+module.exports = { setupQueueMicrotask };
diff --git a/src/node.cc b/src/node.cc
@@ -1423,6 +1423,18 @@ void FatalException(Isolate* isolate, const TryCatch& try_catch) {
 }
 
 
+static void FatalException(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Environment* env = Environment::GetCurrent(isolate);
+  if (env != nullptr && env->abort_on_uncaught_exception()) {
+    Abort();
+  }
+  Local<Value> exception = args[0];
+  Local<Message> message = Exception::CreateMessage(isolate, exception);
+  FatalException(isolate, exception, message);
+}
+
+
 static void OnMessage(Local<Message> message, Local<Value> error) {
   // The current version of V8 sends messages for errors only
   // (thus `error` is always set).
@@ -2161,14 +2173,19 @@ void LoadEnvironment(Environment* env) {
     return;
   }
 
+  Local<Function> trigger_fatal_exception =
+      env->NewFunctionTemplate(FatalException)->GetFunction(env->context())
+          .ToLocalChecked();
+
   // Bootstrap Node.js
   Local<Object> bootstrapper = Object::New(env->isolate());
   SetupBootstrapObject(env, bootstrapper);
   Local<Value> bootstrapped_node;
   Local<Value> node_bootstrapper_args[] = {
     env->process_object(),
     bootstrapper,
-    bootstrapped_loaders
+    bootstrapped_loaders,
+    trigger_fatal_exception,
   };
   if (!ExecuteBootstrapper(env, node_bootstrapper.ToLocalChecked(),
                            arraysize(node_bootstrapper_args),

diff --git a/test/parallel/test-queue-microtask.js b/test/parallel/test-queue-microtask.js
@@ -42,7 +42,7 @@ queueMicrotask(common.mustCall(function() {
 }
 
 const eq = [];
-process.on('error', (e) => {
+process.on('uncaughtException', (e) => {
   eq.push(e);
 });