From 513adc97b5a0202a71a7710ba07678baba7aa5a6 Mon Sep 17 00:00:00 2001
From: Mathieu Hofman <86499+mhofman@users.noreply.github.com>
Date: Fri, 14 Jun 2024 18:30:36 -0700
Subject: [PATCH] refactor(internal): move async helpers using AggregateError
 to node (#9508)

closes: #9504

## Description

While #9275 switched to a global `AggregrateError` constructor, it was unclear if that was safe because that global is not currently available in xsnap. Instead of reverting, this PR moves the "shared" internal helpers that used an `AggregateError` to the `node` folder, to make it clear they should only be used in that environment. All usages of these helpers were already in Node only.

All other usages of `AggregateError` were audited to confirm they only happened on Node as well.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
We're unfortunately lacking coverage of some code under xsnap, Open to ideas on how to improve testing here

It would also be nice somehow to make sure the `node` folders do no end up imported in bundled code. Maybe having a pseudo `import 'node:process'` could work? @kriskowal

### Upgrade Considerations
None
---
 packages/internal/src/node/utils.js        | 46 ++++++++++++++++++++++
 packages/internal/src/utils.js             | 40 -------------------
 packages/swing-store/src/snapStore.js      |  5 ++-
 packages/telemetry/src/make-slog-sender.js |  2 +-
 4 files changed, 51 insertions(+), 42 deletions(-)
 create mode 100644 packages/internal/src/node/utils.js

diff --git a/packages/internal/src/node/utils.js b/packages/internal/src/node/utils.js
new file mode 100644
index 00000000000..ba4995c3798
--- /dev/null
+++ b/packages/internal/src/node/utils.js
@@ -0,0 +1,46 @@
+// @ts-check
+// @jessie-check
+
+// These tools seem cross platform, but they rely on AggregateError in the error
+// handling path, which is currently not available in xsnap
+import 'node:process';
+
+/**
+ * @template T
+ * @param {readonly (T | PromiseLike<T>)[]} items
+ * @returns {Promise<T[]>}
+ */
+export const PromiseAllOrErrors = async items => {
+  return Promise.allSettled(items).then(results => {
+    const errors = /** @type {PromiseRejectedResult[]} */ (
+      results.filter(({ status }) => status === 'rejected')
+    ).map(result => result.reason);
+    if (!errors.length) {
+      return /** @type {PromiseFulfilledResult<T>[]} */ (results).map(
+        result => result.value,
+      );
+    } else if (errors.length === 1) {
+      throw errors[0];
+    } else {
+      throw AggregateError(errors);
+    }
+  });
+};
+
+/**
+ * @type {<T>(
+ *   trier: () => Promise<T>,
+ *   finalizer: (error?: unknown) => Promise<void>,
+ * ) => Promise<T>}
+ */
+export const aggregateTryFinally = async (trier, finalizer) =>
+  trier().then(
+    async result => finalizer().then(() => result),
+    async tryError =>
+      finalizer(tryError)
+        .then(
+          () => tryError,
+          finalizeError => AggregateError([tryError, finalizeError]),
+        )
+        .then(error => Promise.reject(error)),
+  );
diff --git a/packages/internal/src/utils.js b/packages/internal/src/utils.js
index 478b87ec44c..be3cf6dc8a9 100644
--- a/packages/internal/src/utils.js
+++ b/packages/internal/src/utils.js
@@ -74,46 +74,6 @@ export const makeMeasureSeconds = currentTimeMillisec => {
   return measureSeconds;
 };
 
-/**
- * @template T
- * @param {readonly (T | PromiseLike<T>)[]} items
- * @returns {Promise<T[]>}
- */
-export const PromiseAllOrErrors = async items => {
-  return Promise.allSettled(items).then(results => {
-    const errors = /** @type {PromiseRejectedResult[]} */ (
-      results.filter(({ status }) => status === 'rejected')
-    ).map(result => result.reason);
-    if (!errors.length) {
-      return /** @type {PromiseFulfilledResult<T>[]} */ (results).map(
-        result => result.value,
-      );
-    } else if (errors.length === 1) {
-      throw errors[0];
-    } else {
-      throw AggregateError(errors);
-    }
-  });
-};
-
-/**
- * @type {<T>(
- *   trier: () => Promise<T>,
- *   finalizer: (error?: unknown) => Promise<void>,
- * ) => Promise<T>}
- */
-export const aggregateTryFinally = async (trier, finalizer) =>
-  trier().then(
-    async result => finalizer().then(() => result),
-    async tryError =>
-      finalizer(tryError)
-        .then(
-          () => tryError,
-          finalizeError => AggregateError([tryError, finalizeError]),
-        )
-        .then(error => Promise.reject(error)),
-  );
-
 /**
  * @template {Record<string, unknown>} T
  * @typedef {{ [P in keyof T]: Exclude<T[P], undefined> }} AllDefined
diff --git a/packages/swing-store/src/snapStore.js b/packages/swing-store/src/snapStore.js
index 3d67ad503b8..60ae903d899 100644
--- a/packages/swing-store/src/snapStore.js
+++ b/packages/swing-store/src/snapStore.js
@@ -4,7 +4,10 @@ import { finished as finishedCallback, PassThrough, Readable } from 'stream';
 import { promisify } from 'util';
 import { createGzip, createGunzip } from 'zlib';
 import { Fail, q } from '@agoric/assert';
-import { aggregateTryFinally, PromiseAllOrErrors } from '@agoric/internal';
+import {
+  aggregateTryFinally,
+  PromiseAllOrErrors,
+} from '@agoric/internal/src/node/utils.js';
 import { buffer } from './util.js';
 
 /**
diff --git a/packages/telemetry/src/make-slog-sender.js b/packages/telemetry/src/make-slog-sender.js
index 94693661087..5eb56aa65c5 100644
--- a/packages/telemetry/src/make-slog-sender.js
+++ b/packages/telemetry/src/make-slog-sender.js
@@ -1,6 +1,6 @@
 import path from 'path';
 import tmp from 'tmp';
-import { PromiseAllOrErrors } from '@agoric/internal';
+import { PromiseAllOrErrors } from '@agoric/internal/src/node/utils.js';
 import { serializeSlogObj } from './serialize-slog-obj.js';
 
 export const DEFAULT_SLOGSENDER_MODULE =