fix: kickOut does not throw itself (#1663)

* fix: kickOut does not throw itself

* refactor: don't throw if seat that was kickedOut already exited.

* chore: add tests for kicking out multiple seats in one go

* chore: change the tests slightly, remove a console.log that shouldn't be there

* chore: remove t.plan that was there unintentionally from prior tests

packages/zoe/src/contractFacet/contractFacet.js

@@ -368,11 +368,7 @@ export function buildRootObject() {
         const offerHandler = invitationHandleToHandler.get(invitationHandle);
         // @ts-ignore
         const offerResultP = E(offerHandler)(zcfSeat).catch(reason => {
-          if (!zcfSeat.hasExited()) {
-            throw zcfSeat.kickOut(reason);
-          } else {
-            throw reason;
-          }
+          throw zcfSeat.kickOut(reason);
         });
         const exitObj = makeExitObj(
           seatData.proposal,

packages/zoe/src/contractFacet/seat.js

@@ -55,10 +55,11 @@ export const makeZcfSeatAdminKit = (
         'Kicked out of seat. Please check the log for more information.',
       ),
     ) => {
-      assertExitedFalse();
-      zcfSeatAdmin.updateHasExited();
-      E(zoeSeatAdmin).kickOut(harden(reason));
-      throw reason;
+      if (!exited) {
+        zcfSeatAdmin.updateHasExited();
+        E(zoeSeatAdmin).kickOut(harden(reason));
+      }
+      return reason;
     },
     getNotifier: () => {
       return notifier;

packages/zoe/src/contractFacet/types.js

@@ -131,7 +131,7 @@
 /**
  * @typedef {Object} ZCFSeat
  * @property {() => void} exit
- * @property {(reason?: any) => never} kickOut called with the reason this
+ * @property {(reason?: Error) => Error} kickOut called with the reason this
  * seat is being kicked out, where reason is normally an instanceof Error.
  * @property {() => Notifier<Allocation>} getNotifier
  * @property {() => boolean} hasExited

packages/zoe/src/contractSupport/zoeHelpers.js

@@ -165,7 +165,7 @@ export const trade = (zcf, keepLeft, tryRight) => {
     if (!offerSafeForRight) {
       console.log(`offer not safe for right`);
     }
-    return tryRight.seat.kickOut(
+    throw tryRight.seat.kickOut(
       new Error(
         `The trade between left ${keepLeft} and right ${tryRight} failed offer safety. Please check the log for more information`,
       ),

packages/zoe/src/contracts/auction/secondPriceLogic.js

@@ -29,7 +29,9 @@ export const calcWinnerAndClose = (zcf, sellSeat, bidSeats) => {
   });
 
   if (activeBidsCount === 0) {
-    sellSeat.kickOut(new Error(`Could not close auction. No bids were active`));
+    throw sellSeat.kickOut(
+      new Error(`Could not close auction. No bids were active`),
+    );
   }
 
   if (activeBidsCount === 1) {

packages/zoe/src/zoeService/zoe.js

@@ -430,7 +430,6 @@ function makeZoe(vatAdminSvc, zcfBundleName = undefined) {
             })
             .catch(err => {
               console.log(err);
-              console.log('right here');
             });
 
           return userSeat;

packages/zoe/test/unitTests/contracts/test-zcf.js

@@ -3,6 +3,7 @@ import '@agoric/install-ses';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import test from 'ava';
 
+import { E } from '@agoric/eventual-send';
 import bundleSource from '@agoric/bundle-source';
 
 // noinspection ES6PreferShortImport
@@ -12,8 +13,7 @@ import fakeVatAdmin from './fakeVatAdmin';
 
 const contractRoot = `${__dirname}/zcfTesterContract`;
 
-test('zoe - test zcf', async t => {
-  t.plan(1);
+test(`zoe - zcfSeat.kickOut() doesn't throw`, async t => {
   const { moolaIssuer, simoleanIssuer } = setup();
   const zoe = makeZoe(fakeVatAdmin);
 
@@ -27,7 +27,40 @@ test('zoe - test zcf', async t => {
     Pixels: moolaIssuer,
     Money: simoleanIssuer,
   });
-  await t.notThrowsAsync(() =>
-    zoe.startInstance(installation, issuerKeywordRecord),
+
+  const { creatorFacet } = await E(zoe).startInstance(
+    installation,
+    issuerKeywordRecord,
   );
+
+  // This contract gives ZCF as the contractFacet for testing purposes
+  /** @type ContractFacet */
+  const zcf = creatorFacet;
+
+  let firstSeat;
+
+  const grabSeat = seat => {
+    firstSeat = seat;
+    return 'ok';
+  };
+
+  const kickOutSeat = secondSeat => {
+    firstSeat.kickOut(new Error('kicked out first'));
+    throw secondSeat.kickOut(new Error('kicked out second'));
+  };
+
+  const invitation1 = await zcf.makeInvitation(grabSeat, 'seat1');
+  const invitation2 = await zcf.makeInvitation(kickOutSeat, 'seat2');
+
+  const userSeat1 = await E(zoe).offer(invitation1);
+  const userSeat2 = await E(zoe).offer(invitation2);
+
+  t.is(await E(userSeat1).getOfferResult(), 'ok', `userSeat1 offer result`);
+
+  t.deepEqual(await E(userSeat2).getPayouts(), {});
+
+  await t.throwsAsync(E(userSeat2).getOfferResult());
+  await t.throwsAsync(() => E(userSeat1).tryExit(), {
+    message: 'seat has been exited',
+  });
 });

packages/zoe/test/unitTests/contracts/zcfTesterContract.js

@@ -1,14 +1,13 @@
 // @ts-check
 
+import '../../../exported';
+
 /**
  * Tests ZCF
  * @type {ContractStartFn}
  */
-const start = _zcf => {
-  // TODO: import tap/tape and do t.is
-  // TODO: Test ZCF here
-
-  return harden({});
+const start = zcf => {
+  return { creatorFacet: zcf };
 };
 
 harden(start);