async_wrap: return undefined if domain is disposed

v8::MaybeLocal::ToLocalChecked() will abort on an empty handle.
AsyncWrap::MakeCallback returns an empty handle if the domain is
disposed, but this should not cause the process to abort. So instead
return v8::Undefined() and allow the error to be handled normally.

PR-URL: #14722
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>

Author: trevnorris

src/async-wrap.cc

@@ -54,6 +54,7 @@ using v8::String;
 using v8::Symbol;
 using v8::TryCatch;
 using v8::Uint32Array;
+using v8::Undefined;
 using v8::Value;
 
 using AsyncHooks = node::Environment::AsyncHooks;
@@ -696,7 +697,7 @@ MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
                                                 get_trigger_id());
 
   if (!PreCallbackExecution(this, true)) {
-    return MaybeLocal<Value>();
+    return Undefined(env()->isolate());
   }
 
   // Finally... Get to running the user's callback.
@@ -707,7 +708,7 @@ MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   }
 
   if (!PostCallbackExecution(this, true)) {
-    return Local<Value>();
+    return Undefined(env()->isolate());
   }
 
   exec_scope.Dispose();

test/parallel/test-domain-abort-when-disposed.js

@@ -0,0 +1,25 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const domain = require('domain');
+
+// These were picked arbitrarily and are only used to trigger arync_hooks.
+const JSStream = process.binding('js_stream').JSStream;
+const Socket = require('net').Socket;
+
+const handle = new JSStream();
+handle.domain = domain.create();
+handle.domain.dispose();
+
+handle.close = () => {};
+handle.isAlive = () => { throw new Error(); };
+
+const s = new Socket({ handle });
+
+// When AsyncWrap::MakeCallback() returned an empty handle the
+// MaybeLocal::ToLocalChecked() call caused the process to abort. By returning
+// v8::Undefined() it allows the error to propagate to the 'error' event.
+s.on('error', common.mustCall((e) => {
+  assert.strictEqual(e.code, 'EINVAL');
+}));