Skip to content

Commit

Permalink
refactor: add back a try-catch in executeOffer
Browse files Browse the repository at this point in the history
  • Loading branch information
@Chris-Hibbert
Chris-Hibbert committed Nov 1, 2023
1 parent 2d88502 commit b5093a4
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 60 deletions.
123 changes: 75 additions & 48 deletions packages/smart-wallet/src/smartWallet.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -735,61 +735,88 @@ export const prepareSmartWallet = (baggage, shared) => {

facets.helper.assertUniqueOfferId(String(offerSpec.id));

const invitationFromSpec = makeInvitationsHelper(
zoe,
agoricNames,
invitationBrand,
invitationPurse,
state.offerToInvitationMakers.get,
);

console.info(
'wallet',
address,
'starting executeOffer',
offerSpec.id,
);

// 1. Prepare values and validate synchronously.
const { proposal } = offerSpec;
let seatRef;
let watcher;
try {
const invitationFromSpec = makeInvitationsHelper(
zoe,
agoricNames,
invitationBrand,
invitationPurse,
state.offerToInvitationMakers.get,
);

const invitation = invitationFromSpec(offerSpec.invitationSpec);
console.info(
'wallet',
address,
'starting executeOffer',
offerSpec.id,
);

const [paymentKeywordRecord, invitationAmount] = await Promise.all([
proposal?.give &&
deeplyFulfilledObject(
facets.payments.withdrawGive(proposal.give),
),
E(invitationIssuer).getAmountOf(invitation),
]);
// 1. Prepare values and validate synchronously.
const { proposal } = offerSpec;

// 2. Begin executing offer
// No explicit signal to user that we reached here but if anything above
// failed they'd get an 'error' status update.
const invitation = invitationFromSpec(offerSpec.invitationSpec);

/** @type {UserSeat} */
const seatRef = await E(zoe).offer(
invitation,
proposal,
paymentKeywordRecord,
offerSpec.offerArgs,
);
const [paymentKeywordRecord, invitationAmount] = await Promise.all([

Check failure on line 761 in packages/smart-wallet/src/smartWallet.js

View workflow job for this annotation

GitHub Actions / lint-rest 

The first `await` appearing in an async function must not be nested
proposal?.give &&
deeplyFulfilledObject(
facets.payments.withdrawGive(proposal.give),
),
E(invitationIssuer).getAmountOf(invitation),
]);

// 2. Begin executing offer
// No explicit signal to user that we reached here but if anything above
// failed they'd get an 'error' status update.

/** @type {UserSeat} */
seatRef = await E(zoe).offer(
invitation,
proposal,
paymentKeywordRecord,
offerSpec.offerArgs,
);
console.info('wallet', address, offerSpec.id, 'seated');

console.info('wallet', address, offerSpec.id, 'seated');
watcher = makeOfferWatcher(
facets.helper,
facets.deposit,
offerSpec,
address,
invitationAmount,
seatRef,
);

const watcher = makeOfferWatcher(
facets.helper,
facets.deposit,
offerSpec,
address,
invitationAmount,
seatRef,
);
watchOfferOutcomes(watcher, seatRef);
state.liveOffers.init(offerSpec.id, offerSpec);
facets.helper.publishCurrentState();
state.liveOfferSeats.init(offerSpec.id, seatRef);
} catch (err) {
console.error('OFFER ERROR:', err);
// Notify the user
debugger;

Check failure on line 798 in packages/smart-wallet/src/smartWallet.js

View workflow job for this annotation

GitHub Actions / lint-rest 

Unexpected 'debugger' statement
if (watcher) {
watcher.helper.updateStatus({ error: err.toString() });
} else {
facets.helper.updateStatus(
{
error: err.toString(),
...offerSpec,
},
address,
);
}

watchOfferOutcomes(watcher, seatRef);
state.liveOffers.init(offerSpec.id, offerSpec);
facets.helper.publishCurrentState();
state.liveOfferSeats.init(offerSpec.id, seatRef);
if (seatRef) {
E.when(E(seatRef).hasExited(), hasExited => {

Check failure on line 812 in packages/smart-wallet/src/smartWallet.js

View workflow job for this annotation

GitHub Actions / lint-rest 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
if (!hasExited) {
void E(seatRef).tryExit();
}
});
}
throw err;
}
},
/**
* Take an offer's id, look up its seat, try to exit.
Expand Down
37 changes: 25 additions & 12 deletions packages/smart-wallet/test/test-addAsset.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import bundleSource from '@endo/bundle-source';
import { makeMarshal } from '@endo/marshal';
import { resolve as importMetaResolve } from 'import-meta-resolve';
import { makeDefaultTestContext } from './contexts.js';
import { ActionType, headValue, makeMockTestSpace } from './supports.js';
import { ActionType, makeMockTestSpace } from './supports.js';
import { makeImportContext } from '../src/marshal-contexts.js';

const { Fail } = assert;
const { Fail, quote: q } = assert;

const importSpec = spec =>
importMetaResolve(spec, import.meta.url).then(u => new URL(u).pathname);
Expand Down Expand Up @@ -376,8 +376,10 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => {
);
const wallet = simpleProvideWallet(addr);
t.log('deposit', istQty, 'IST into wallet of', addr);
await E(E(wallet).getDepositFacet()).receive(spendingPmt);

// order of updates changed so offerStatus might not be last
const updates = await E(wallet).getUpdatesSubscriber();
await E(E(wallet).getDepositFacet()).receive(spendingPmt);
return updates;
};

Expand Down Expand Up @@ -418,10 +420,21 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => {
['Boardwalk', 1n],
];

/** @type {import('../src/smartWallet.js').UpdateRecord} */
const update = await headValue(updates);
assert(update.updated === 'offerStatus');
// t.log(update.status);
// status update used to be last, but now we have to look for the right one
let record = await E(updates).subscribeAfter();
while (
record.head.value.updated !== 'offerStatus' ||
!record.head.value.status.numWantsSatisfied ||
!record.head.value.status.payouts?.Places
) {
record = await E(updates).subscribeAfter(record.publishCount);
}
const update = record.head.value;

assert(
update.updated === 'offerStatus',
`Should have had "updated":"offerStatus", had "${q(update)}"`,
);
t.like(update, {
updated: 'offerStatus',
status: {
Expand All @@ -435,7 +448,6 @@ test.serial('trading in non-vbank asset: game real-estate NFTs', async t => {
const {
status: { id, result, payouts },
} = update;
// @ts-expect-error cast value to copyBag
const names = payouts?.Places.value.payload.map(([name, _qty]) => name);
t.log(id, 'result:', result, ', payouts:', names.join(', '));

Expand Down Expand Up @@ -495,13 +507,15 @@ test.serial('non-vbank asset: give before deposit', async t => {
proposal: { give, want },
});
t.log('goofy client: propose to give', choices.join(', '));
await E(walletBridge).proposeOffer(ctx.fromBoard.toCapData(offer1));
await t.throwsAsync(
() => E(walletBridge).proposeOffer(ctx.fromBoard.toCapData(offer1)),
{ message: /Withdrawal of .* failed because the purse only contained/ },
);
};

{
const addr2 = 'agoric1player2';
const walletUIbridge = makePromiseKit();
// await eventLoopIteration();

const { simpleProvideWallet, consume, sendToBridge } = t.context;
const wallet = simpleProvideWallet(addr2);
Expand All @@ -512,8 +526,7 @@ test.serial('non-vbank asset: give before deposit', async t => {
const { aPlayer } = makeScenario(t);

await aPlayer(addr2, walletUIbridge, mockStorage, sendToBridge, updates);
const c2 = goofyClient(mockStorage, walletUIbridge.promise);
await t.throwsAsync(c2, { message: /Withdrawal of {.*} failed/ });
await goofyClient(mockStorage, walletUIbridge.promise);
await eventLoopIteration();

// wallet balance was also updated
Expand Down

0 comments on commit b5093a4

Please sign in to comment.