lib: comment explaining special-case handling of promises

Promise handling is special-cased for domains and trace_events. Domains must not add the `domain` property to promises in other contexts, and trace_events must be emitted from JavaScript as promises no longer have an AsyncWrap type attached to them. PR-URL: nodejs#39135 Backport-PR-URL: nodejs#39742 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>
foxxyz · Oct 18, 2021 · 8f24614 · 8f24614
1 parent 0dc51c1
commit 8f24614
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 0 deletions.
diff --git a/lib/domain.js b/lib/domain.js
@@ -74,6 +74,8 @@ const asyncHook = createHook({
     if (process.domain !== null && process.domain !== undefined) {
       // If this operation is created while in a domain, let's mark it
       pairing.set(asyncId, process.domain[kWeak]);
+      // Promises from other contexts, such as with the VM module, should not
+      // have a domain property as it can be used to escape the sandbox.
       if (type !== 'PROMISE' || resource instanceof Promise) {
         ObjectDefineProperty(resource, 'domain', {
           configurable: true,

diff --git a/lib/internal/trace_events_async_hooks.js b/lib/internal/trace_events_async_hooks.js
@@ -31,6 +31,8 @@ const kEnabled = Symbol('enabled');
 // twice the async_wrap.Providers list is used to filter the events.
 const nativeProviders = new SafeSet(ObjectKeys(async_wrap.Providers));
 const typeMemory = new SafeMap();
+
+// Promises are not AsyncWrap resources so they should emit trace_events here.
 nativeProviders.delete('PROMISE');
 
 function createHook() {