fix: add TODOs for early synchronous returns

Agoric · Jun 7, 2024 · 9428ec1 · 9428ec1
1 parent e5a1897
commit 9428ec1
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 18 deletions.
diff --git a/packages/orchestration/src/exos/chainAccountKit.js b/packages/orchestration/src/exos/chainAccountKit.js
@@ -64,7 +64,7 @@ export const prepareChainAccountKit = (zone, { watch, when }) =>
     {
       account: ChainAccountI,
       connectionHandler: ConnectionHandlerI,
-      handleExecuteEncodedTxWatcher: M.interface('HandleQueryWatcher', {
+      parseTxPacketWatcher: M.interface('ParseTxPacketWatcher', {
         onFulfilled: M.call(M.string())
           .optional(M.arrayOf(M.undefined())) // does not need watcherContext
           .returns(M.string()),
@@ -84,7 +84,7 @@ export const prepareChainAccountKit = (zone, { watch, when }) =>
         localAddress: undefined,
       }),
     {
-      handleExecuteEncodedTxWatcher: {
+      parseTxPacketWatcher: {
         /** @param {string} ack */
         onFulfilled(ack) {
           return parseTxPacket(ack);
@@ -136,11 +136,13 @@ export const prepareChainAccountKit = (zone, { watch, when }) =>
          */
         executeEncodedTx(msgs, opts) {
           const { connection } = this.state;
+          // TODO #9281 do not throw synchronously when returning a promise; return a rejected Vow
+          /// see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694
           if (!connection) throw Fail`connection not available`;
           return when(
             watch(
               E(connection).send(makeTxPacket(msgs, opts)),
-              this.facets.handleExecuteEncodedTxWatcher,
+              this.facets.parseTxPacketWatcher,
             ),
           );
         },
@@ -150,6 +152,8 @@ export const prepareChainAccountKit = (zone, { watch, when }) =>
           // - retrieve assets?
           // - revoke the port?
           const { connection } = this.state;
+          // TODO #9281 do not throw synchronously when returning a promise; return a rejected Vow
+          /// see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694
           if (!connection) throw Fail`connection not available`;
           return when(watch(E(connection).close()));
         },

diff --git a/packages/orchestration/src/exos/icqConnectionKit.js b/packages/orchestration/src/exos/icqConnectionKit.js
@@ -29,12 +29,6 @@ export const ICQConnectionI = M.interface('ICQConnection', {
   query: M.call(M.arrayOf(ICQMsgShape)).returns(M.promise()),
 });
 
-const HandleQueryWatcherI = M.interface('HandleQueryWatcher', {
-  onFulfilled: M.call(M.string())
-    .optional(M.arrayOf(M.undefined())) // does not need watcherContext
-    .returns(M.arrayOf(M.record())),
-});
-
 /**
  * @typedef {{
  *   port: Port;
@@ -65,8 +59,12 @@ export const prepareICQConnectionKit = (zone, { watch, when }) =>
     'ICQConnectionKit',
     {
       connection: ICQConnectionI,
-      handleQueryWatcher: HandleQueryWatcherI,
       connectionHandler: ConnectionHandlerI,
+      parseQueryPacketWatcher: M.interface('ParseQueryPacketWatcher', {
+        onFulfilled: M.call(M.string())
+          .optional(M.arrayOf(M.undefined())) // does not need watcherContext
+          .returns(M.arrayOf(M.record())),
+      }),
     },
     /** @param {Port} port */
     port =>
@@ -97,16 +95,18 @@ export const prepareICQConnectionKit = (zone, { watch, when }) =>
          */
         query(msgs) {
           const { connection } = this.state;
+          // TODO #9281 do not throw synchronously when returning a promise; return a rejected Vow
+          /// see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694
           if (!connection) throw Fail`connection not available`;
           return when(
             watch(
               E(connection).send(makeQueryPacket(msgs)),
-              this.facets.handleQueryWatcher,
+              this.facets.parseQueryPacketWatcher,
             ),
           );
         },
       },
-      handleQueryWatcher: {
+      parseQueryPacketWatcher: {
         /** @param {string} ack packet acknowledgement string */
         onFulfilled(ack) {
           return parseQueryPacket(ack);

diff --git a/packages/orchestration/src/service.js b/packages/orchestration/src/service.js
@@ -69,12 +69,12 @@ const prepareOrchestrationKit = (
   zone.exoClassKit(
     'Orchestration',
     {
-      requestICAConnectionWatcher: M.interface('RequestICAConnectionWatcher', {
+      requestICAChannelWatcher: M.interface('RequestICAChannelWatcher', {
         onFulfilled: M.call(M.remotable('Port'))
           .optional({ remoteConnAddr: M.string() })
           .returns(NetworkShape.Vow$(NetworkShape.Connection)),
       }),
-      requestICQConnectionWatcher: M.interface('RequestICQConnectionWatcher', {
+      requestICQChannelWatcher: M.interface('RequestICQChannelWatcher', {
         onFulfilled: M.call(M.remotable('Port'))
           .optional({
             remoteConnAddr: M.string(),
@@ -114,7 +114,7 @@ const prepareOrchestrationKit = (
       return /** @type {OrchestrationState} */ ({ powers, icqConnections });
     },
     {
-      requestICAConnectionWatcher: {
+      requestICAChannelWatcher: {
         /**
          * @param {Port} port
          * @param {{
@@ -130,7 +130,7 @@ const prepareOrchestrationKit = (
           );
         },
       },
-      requestICQConnectionWatcher: {
+      requestICQChannelWatcher: {
         /**
          * @param {Port} port
          * @param {{
@@ -197,7 +197,7 @@ const prepareOrchestrationKit = (
           return when(
             watch(
               E(portAllocator).allocateICAControllerPort(),
-              this.facets.requestICAConnectionWatcher,
+              this.facets.requestICAChannelWatcher,
               {
                 remoteConnAddr,
               },
@@ -210,6 +210,7 @@ const prepareOrchestrationKit = (
          */
         provideICQConnection(controllerConnectionId) {
           if (this.state.icqConnections.has(controllerConnectionId)) {
+            // TODO #9281 do not return synchronously. see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694
             return this.state.icqConnections.get(controllerConnectionId)
               .connection;
           }
@@ -220,7 +221,7 @@ const prepareOrchestrationKit = (
               // allocate a new Port for every Connection
               // TODO #9317 optimize ICQ port allocation
               E(portAllocator).allocateICQControllerPort(),
-              this.facets.requestICQConnectionWatcher,
+              this.facets.requestICQChannelWatcher,
               {
                 remoteConnAddr,
                 controllerConnectionId,

diff --git a/packages/orchestration/test/service.test.ts b/packages/orchestration/test/service.test.ts
@@ -38,6 +38,12 @@ test('makeICQConnection returns an ICQConnection', async t => {
     'remote address contains icqhost port, unordered ordering, and icq-1 version string',
   );
 
+  const icqConnection2 = await E(orchestration).provideICQConnection(
+    CONTROLLER_CONNECTION_ID,
+  );
+  const localAddr2 = await E(icqConnection2).getLocalAddress();
+  t.is(localAddr, localAddr2, 'provideICQConnection is idempotent');
+
   await t.throwsAsync(
     E(icqConnection).query([
       toRequestQueryJson(