From 205e45f9adc43a795d4689627c830ba0ee9178f9 Mon Sep 17 00:00:00 2001
From: "Mark S. Miller" <erights@users.noreply.github.com>
Date: Fri, 5 Apr 2024 13:54:13 -0700
Subject: [PATCH] fix(pass-style): toPassableError fixed.
 (is/assert)PassableError removed. (#2156)

---
 packages/pass-style/index.js                  |  2 -
 packages/pass-style/src/passStyleOf.js        | 49 +++----------------
 ...test-extended-errors.js => test-errors.js} | 23 ++++++++-
 3 files changed, 30 insertions(+), 44 deletions(-)
 rename packages/pass-style/test/{test-extended-errors.js => test-errors.js} (53%)

diff --git a/packages/pass-style/index.js b/packages/pass-style/index.js
index 695ef755e0..f841e22a37 100644
--- a/packages/pass-style/index.js
+++ b/packages/pass-style/index.js
@@ -28,8 +28,6 @@ export {
   passStyleOf,
   isPassable,
   assertPassable,
-  isPassableError,
-  assertPassableError,
   toPassableError,
 } from './src/passStyleOf.js';
 
diff --git a/packages/pass-style/src/passStyleOf.js b/packages/pass-style/src/passStyleOf.js
index c1603c5ca7..d96528314a 100644
--- a/packages/pass-style/src/passStyleOf.js
+++ b/packages/pass-style/src/passStyleOf.js
@@ -4,12 +4,7 @@
 
 import { isPromise } from '@endo/promise-kit';
 import { X, Fail, q, annotateError, makeError } from '@endo/errors';
-import {
-  assertChecker,
-  isObject,
-  isTypedArray,
-  PASS_STYLE,
-} from './passStyle-helpers.js';
+import { isObject, isTypedArray, PASS_STYLE } from './passStyle-helpers.js';
 
 import { CopyArrayHelper } from './copyArray.js';
 import { CopyRecordHelper } from './copyRecord.js';
@@ -270,46 +265,19 @@ harden(isPassable);
  */
 const isPassableErrorPropertyDesc = (name, desc) =>
   checkRecursivelyPassableErrorPropertyDesc(name, desc, passStyleOf);
-harden(isPassableErrorPropertyDesc);
-
-/**
- * @param {string} name
- * @param {PropertyDescriptor} desc
- */
-const assertPassableErrorPropertyDesc = (name, desc) => {
-  checkRecursivelyPassableErrorPropertyDesc(
-    name,
-    desc,
-    passStyleOf,
-    assertChecker,
-  );
-};
-harden(assertPassableErrorPropertyDesc);
-
-/**
- * @param {unknown} err
- * @returns {err is Error}
- */
-export const isPassableError = err =>
-  checkRecursivelyPassableError(err, passStyleOf);
-
-/**
- * @param {unknown} err
- * @returns {asserts err is Error}
- */
-export const assertPassableError = err => {
-  checkRecursivelyPassableError(err, passStyleOf, assertChecker);
-};
 
 /**
- * Return a new passable error that propagates the diagnostic info of the
+ * Return a passable error that propagates the diagnostic info of the
  * original, and is linked to the original as a note.
+ * `toPassableError` hardens the argument before checking if it is already
+ * a passable error. If it is, then `toPassableError` returns the argument.
  *
  * @param {Error} err
  * @returns {Error}
  */
 export const toPassableError = err => {
-  if (isPassableError(err)) {
+  harden(err);
+  if (checkRecursivelyPassableError(err, passStyleOf)) {
     return err;
   }
   const { name, message } = err;
@@ -318,11 +286,9 @@ export const toPassableError = err => {
   let cause;
   let errors;
   if (causeDesc && isPassableErrorPropertyDesc('cause', causeDesc)) {
-    // @ts-expect-error data descriptors have "value" property
     cause = causeDesc.value;
   }
   if (errorsDesc && isPassableErrorPropertyDesc('errors', errorsDesc)) {
-    // @ts-expect-error data descriptors have "value" property
     errors = errorsDesc.value;
   }
 
@@ -337,7 +303,8 @@ export const toPassableError = err => {
   // cause hidden diagnostic information of the original error
   // to be logged.
   annotateError(newError, X`copied from error ${err}`);
-  assertPassableError(newError);
+  passStyleOf(newError) === 'error' ||
+    Fail`Expected ${newError} to be a passable error`;
   return newError;
 };
 harden(toPassableError);
diff --git a/packages/pass-style/test/test-extended-errors.js b/packages/pass-style/test/test-errors.js
similarity index 53%
rename from packages/pass-style/test/test-extended-errors.js
rename to packages/pass-style/test/test-errors.js
index 11d02b2f79..b1e0e44a70 100644
--- a/packages/pass-style/test/test-extended-errors.js
+++ b/packages/pass-style/test/test-errors.js
@@ -1,7 +1,12 @@
 /* eslint-disable max-classes-per-file */
 import test from '@endo/ses-ava/prepare-endo.js';
 
-import { passStyleOf } from '../src/passStyleOf.js';
+import { makeError } from '@endo/errors';
+import {
+  passStyleOf,
+  isPassable,
+  toPassableError,
+} from '../src/passStyleOf.js';
 
 test('style of extended errors', t => {
   const e1 = Error('e1');
@@ -23,3 +28,19 @@ test('style of extended errors', t => {
     t.is(passStyleOf(a4), 'error');
   }
 });
+
+test('toPassableError rejects unfrozen errors', t => {
+  const e = makeError('test error');
+  // I include this test because I was recently surprised that the errors
+  // make by `makeError` are not frozen, and therefore not passable.
+  t.false(Object.isFrozen(e));
+  t.false(isPassable(e));
+
+  // toPassableError hardens, and then checks whether the hardened argument
+  // is a passable error.
+  const e2 = toPassableError(e);
+
+  t.is(e, e2);
+  t.true(Object.isFrozen(e));
+  t.true(isPassable(e));
+});