Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

async flow should support cause chains on errors #10375

Open mhofman opened 2 days ago

mhofman commented 2 days ago

What is the Problem Being Solved?

We're now allowing nested data in errors. While async flow does not enforce that errors have the same message, we should enforce that any other nested data, at least cause, is the same when matching the replay log.

This is in particular useful for our plan of using nested cause for upgrade disconnection. See https://github.com/Agoric/agoric-sdk/issues/9582

Async flow is actually in a particularly interesting position to implement #9582 within its membrane. #10147 is similarly adding async stack traces, but it cannot do that for upgrade disconnection reasons. The async flow membrane could replace the upgrade disconnection reason with an error with the disconnection reason as cause, and attach stack information to it. That requires updating the isUpdateDisconnectionReason to walk the cause chain verifying that the leaf is a disconnection reason.

The main concern is if this error thrown into the guest bubbles back out of the membrane through the result into a vat that uses an older predicate that doesn't support this cause walk. Arguably this fine because this error is terminal anyway (async flow's watch has already decided it wasn't actionable if it let it through to the guest).

Description of the Design

Security Considerations

None?

Scaling Considerations

TBD

Test Plan

Throw an error or rejected promise with an error that has an arbitrarily deeply nested cause with an upgrade disconnection reason at its leaf

Upgrade Considerations

Assess compatibility of wrapped errors bubbling out of the membrane

erights commented 2 days ago

See also https://github.com/endojs/endo/pull/2223

Note that the JS standard now has 4 cause-ish Error properties:

which raises the questions