make userSeat.getPayout(foo) throw if foo is not present

[Chris-Hibbert]

closes: #8610

Description

#8610 explains that getPayout(foo) returns undefined, which doesn't match the declarations. I explored changing the types to indicate that undefined is possible, but that would have required that many tests override the types.

It seems cleaner to change the implementation to throw if the named key doesn't correspond to a payment. Callers shouldn't make that mistake outside of tests. If they're uncertain they should call getPayouts() instead.

Security Considerations

No impact

Scaling Considerations

Not relevant

Documentation Considerations

I update the declaration in types-ambient.js, so the generated docs will be updated automatically. I'll create a separate PR to update github.com/Agoric/documentation.

Testing Considerations

Added a test that shows the throw. Nothing else was impacted.

Upgrade Considerations

No relevance.

[cloudflare-workers-and-pages]

Deploying agoric-sdk with    Cloudflare Pages

Latest commit:	64b3b38
Status:	✅  Deploy successful!
Preview URL:	https://a5327a97.agoric-sdk.pages.dev
Branch Preview URL:	https://8610-getpayout-type.agoric-sdk.pages.dev

View logs

[turadg]

Design change LGTM but I'd want to hear from @erights too

[turadg]

Please use NonNullish() instead of a new function

[erights]

When switching to NonNullish, please remember to use the second optional optDetails argument so the error message remains informative.

[erights]

LGTM. Yes, as a general principle we should prefer errors rather than nullish for failures that most likely indicate that there's a bug somewhere that needs to be fixed.

[erights]

When switching to NonNullish, please remember to use the second optional optDetails argument so the error message remains informative.

[erights]

#8610 explains that getPayout(foo) returns null,

Please correct this PR comment before merging.

I looked, and #8610 correctly explains that it returned undefined for that case. Please always be careful about the distinctions between null, undefined, nullish (null or undefined), and falsy. The manual test you're currently doing here

state.payouts[keyword] || Fail`No payout for ${keyword}`;

(that NonNullish will replace) is testing for falsy. NonNullish, as its name correctly implies, tests for nullish. As does the new ?? operator, introduced as an alternative to || for precisely this reason.

'' || 3; // 3
'' ?? 3; // ''

Yes, it sucks that JS forces all these usually-pointless distinctions on us. But given that it does, we need to avoid any further muddying of these treacherous waters.