In vscode debug session, node async_hooks promise keys appear too late

[erights]

Describe the bug

Node's async_hooks apparently adds symbol-named own properties to Promise.prototype and to (some?) individual promises. For passStyleOf to judge a promise passable, ideally it should not have any own properties. However, to accommodate the own properties added by Node's async_hooks to an individual promise, checkPromiseOwnKeys allows symbol-named own properties if they also already appear on Promise.prototype and the properties are visibly safe enough(*).

However, under the vscode debugger, entered using vscode JavaScript Debug Terminal, I have several times run into the

Must not have any own properties: ["[Symbol(async_id_symbol)]","[Symbol(trigger_async_id_symbol)]"]

error, apparently because async_hooks added these to an individual promise instance but not to Promise.prototype.

(*) This should test that the property is a non-configurable non-writable own data property with a visibly harmless value, such as a primitive. However, currently the test seems weaker than that and should be tightened.

Steps to reproduce

Check out a current endo and yarn && yarn build

In vscode, set a breakpoint at

endo/packages/ses/src/error/assert.js

Line 373 in 895ea0a

return error;

  // The next line is a particularly fruitful place to put a breakpoint.
  return error;

In a vscode JavaScript Debug Terminal, run

$ cd packages/cli
$ yarn test test/demo/index.test.js

If the debugger then runs until it stops in the state captured below, you've reproduced the problem.

(Note: Currently speculative that this happens on endo master. Currently I'm on a draft branch with changes that should make no difference. But not sure.)

Expected behavior

Not hitting this error

Platform environment

• What OS are you using?
  • MacOS 15.1.1 (24B91)
• What version of Node.js?
  • v18.19.0
• Is there anything special/unusual about your platform?
  • not that I can think of, that could possibly be relevant.
• What version of Endo are you using? (run git describe --tags --always)
  • @endo/[email protected]

Additional context

I am currently working on a local draft of the markm-flip-non-trapping-shim-default branch. I will check later if this also happens on master.

Screenshots

cli(markm-flip-non-trapping-shim-default)$ yarn test test/demo/index.test.js

Error: "[Promise]" - Must not have any own properties: ["[Symbol(async_id_symbol)]","[Symbol(trigger_async_id_symbol)]"]
  at makeError (file:///Users/markmiller/src/ongithub/endojs/endo/packages/ses/src/error/assert.js:359:61)
  at fail (file:///Users/markmiller/src/ongithub/endojs/endo/packages/ses/src/error/assert.js:491:20)
  at baseAssert (file:///Users/markmiller/src/ongithub/endojs/endo/packages/ses/src/error/assert.js:513:13)
  at assertChecker (file:///Users/markmiller/src/ongithub/endojs/endo/packages/pass-style/src/passStyle-helpers.js:78:3)
  at reject (file:///Users/markmiller/src/ongithub/endojs/endo/packages/pass-style/src/passStyle-helpers.js:93:34)
  at checkPromiseOwnKeys (file:///Users/markmiller/src/ongithub/endojs/endo/packages/pass-style/src/safe-promise.js:42:6)

const checkPromiseOwnKeys = (pr, check) => {
  const keys = ownKeys(pr);

  if (keys.length === 0) {
    return true;
  }

  /**
   * This excludes those symbol-named own properties that are also found on
   * `Promise.prototype`, so that overrides of these properties can be
   * explicitly tolerated if they pass the `checkSafeOwnKey` check below.
   * In particular, we wish to tolerate
   *   * An overriding `toStringTag` non-enumerable data property
   *     with a string value.
   *   * Those own properties that might be added by Node's async_hooks.
   */
  const unknownKeys = keys.filter(
    key => typeof key !== 'symbol' || !hasOwnPropertyOf(Promise.prototype, key),
  );

  if (unknownKeys.length !== 0) {
    return CX(
      check,
    )`${pr} - Must not have any own properties: ${q(unknownKeys)}`;
  }
  // ...
};

In the vscode Debug Console when looking at the checkPromiseOwnKeys stack frame

Object.getOwnPropertyDescriptors(Promise.prototype)
{constructor: {…}, then: {…}, catch: {…}, finally: {…}, Symbol(Symbol.toStringTag): {…}}
  catch = {value: ƒ, writable: false, enumerable: false, configurable: false}
  constructor = {get: ƒ, set: ƒ, enumerable: false, configurable: false}
  finally = {value: ƒ, writable: false, enumerable: false, configurable: false}
  Symbol(Symbol.toStringTag) = {value: 'Promise', writable: false, enumerable: false, configurable: false}
  then = {value: ƒ, writable: false, enumerable: false, configurable: false}
  toString = ƒ [prop]() {\n            return value;\n          }
  valueOf = ƒ [prop]() {\n            return value;\n          }

Object.getOwnPropertyDescriptors(pr)
{Symbol(async_id_symbol): {…}, Symbol(trigger_async_id_symbol): {…}}
  Symbol(async_id_symbol) = {value: 15589, writable: false, enumerable: true, configurable: false}
  Symbol(trigger_async_id_symbol) = {value: 15579, writable: false, enumerable: true, configurable: false}
  toString = ƒ [prop]() {\n            return value;\n          }
  valueOf = ƒ [prop]() {\n            return value;\n          }

[erights]

Verfied to happen on current endo master

$ git describe --tags --always
@endo/[email protected]

[mhofman]

Node's async_hooks apparently adds symbol-named own properties to Promise.prototype

Node doesn't add anything to the prototype, it's our patch that does that.

apparently because async_hooks added these to an individual promise instance but not to Promise.prototype.

That is very surprising. It'd mean that our patch somehow didn't run.

[mhofman]

Ok the problem is that daemon-node.js does not use @endo/init but ses explicitely and @endo/lockdown/commit.js. As such it's skipping the async_hooks patch that is part of @endo/init. @kriskowal any reason to decompose endo init into its parts here?

[mhofman]

The following seems to fix the test above:

diff --git a/packages/daemon/src/daemon-node.js b/packages/daemon/src/daemon-node.js
index 4c84c5502..8a7090ce0 100644
--- a/packages/daemon/src/daemon-node.js
+++ b/packages/daemon/src/daemon-node.js
@@ -2,10 +2,7 @@
 /* global process */
 
 // Establish a perimeter:
-import 'ses';
-import '@endo/eventual-send/shim.js';
-import '@endo/promise-kit/shim.js';
-import '@endo/lockdown/commit.js';
+import '@endo/init';
 
 import crypto from 'crypto';
 import net from 'net';
diff --git a/packages/daemon/src/worker-node.js b/packages/daemon/src/worker-node.js
index b0ca493c0..c61b2e31b 100644
--- a/packages/daemon/src/worker-node.js
+++ b/packages/daemon/src/worker-node.js
@@ -2,10 +2,7 @@
 /* global process */
 
 // Establish a perimeter:
-import 'ses';
-import '@endo/eventual-send/shim.js';
-import '@endo/promise-kit/shim.js';
-import '@endo/lockdown/commit.js';
+import '@endo/init';
 
 import fs from 'fs';
 import url from 'url';

[erights]

@mhofman , #2708 implements your suggestion. The PR comment raises some questions I have about this approach.

[mhofman]

Based on our discussion I filed #2711