Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Orchestration tests have unhandled vow rejections #11026

Open
michaelfig opened this issue Feb 20, 2025 · 2 comments
Open

Orchestration tests have unhandled vow rejections #11026

michaelfig opened this issue Feb 20, 2025 · 2 comments
Assignees
Labels
bug Something isn't working orchestration The Agoric Orchestration Platform

Comments

@michaelfig
Copy link
Member

Describe the bug

While working on #10686 (convert unhandled vow rejections to unhandled promise rejections), we discovered that a few orchestration tests started to fail with unhandled promise rejections.

To Reproduce

Steps to reproduce the behavior:

  1. Search for the use of expectUnhandled( in test files of agoric-sdk/packages/orchestration.
  2. You'll find it used in a few places:
$ cd agoric-sdk/packages/orchestration
$ rg 'expectUnhandled\('
test/exos/local-orchestration-account-kit.test.ts
118:test(expectUnhandled(1), 'transfer', async t => {

test/examples/staking-combinations.test.ts
35:test(expectUnhandled(1), 'start', async t => {

test/examples/send-anywhere.test.ts
260:test(expectUnhandled(1), 'failed ibc transfer returns give', async t => {
$
  1. If you run those tests individually, unhandled promise rejections are reported by AVA.
  2. The use of expectUnhandled allows the tests to return success, even though AVA would normally fail.

I would expect these calls to the expectUnhandled(N) macros to be unnecessary.

Platform Environment

@michaelfig michaelfig added bug Something isn't working orchestration The Agoric Orchestration Platform labels Feb 20, 2025
@michaelfig
Copy link
Member Author

michaelfig commented Feb 20, 2025

It looks like something in the mocked transfer timeout pathway (a rejected vow instead of a fulfilment ack) is not being handled by the orchestration package.

0xpatrickdev added a commit that referenced this issue Feb 26, 2025
- for `LocalOrchAccount.MsgTransfer`, a timeout does not manifest as an immediate rejection of the outgoing message over the vlocalchain bridge. instead, a `VTRANSFER_IBC_EVENT` will come with `event: "timeoutPacket"` over the vtransfer bridge.
- this WIP fixes one of the three `expectUnhandled` in #11026. remaining work is to refactor to move the incoming vtransfer bridge message logic to the test file instead of the vlocalchain bridge handler.
@0xpatrickdev
Copy link
Member

@michaelfig thanks for the pairing today.

The 3 unhandled rejections are isolated to three places where we are incorrectly simulating a MsgTransfer timeout for a local-orchestration-account. We should have been simulating the timeout using the vtransfer bridge.

This commit fixes one of the tests, and leads us (+ @michaelfig ) to believe a MsgTransfer timeout will properly propagate as a Vow rejection for localOrchAccount.transfer(). As such, there's no impact for production.

I've relabelled this issue as a P2. The above commit needs some refactoring (alluded to in the commit message) before it can land on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working orchestration The Agoric Orchestration Platform
Projects
None yet
Development

No branches or pull requests

2 participants