search

Agoric / dapp-agoric-basics

This is a simple app for the Agoric smart contract platform.
0 stars 3 forks source link

Only `then` is special. `catch` and `finally` may validly be in target's method names #30

Open erights opened 3 months ago

erights commented 3 months ago

https://github.com/Agoric/dapp-agoric-basics/blob/5d800a05eae8b77118ee9a8a27e2943fd91dee5c/contract/tools/ui-kit-goals/name-service-client.js#L42

This pmethods list is used to determine whether an invocation should be treated as calling a method on the target, or treated as a meta-level method for talking to the proxy about the target. However, only then is special. catch and finally may validly be in target's method names. @endo/pass-style only enforces that remotables cannot have their own then method. It has no such rule for catch and finally.

@michaelfig , I'm co-assigning this to you, because it looks like the origin is in your draft o package, where this likely also needs to be fixed. (Or is already fixed?)

dckc commented 3 months ago

not yet "fixed" https://github.com/endojs/endo/blob/mfig-o/packages/o/index.js#L44

it's not clear to me why this should be constrained by @endo/pass-style, though.

OTOH, I don't think I depend on using catch or finally here.

erights commented 3 months ago

it's not clear to me why this should be constrained by @endo/pass-style, though.

What other excuse is there for not honoring method names of the target?

michaelfig commented 3 months ago

it's not clear to me why this should be constrained by @endo/pass-style, though.

What other excuse is there for not honoring method names of the target?

The excuse is that the target, in this case, is hidden behind a Promise facade. Arguably, users may want to deal with a node as if it is actually a promise already (node.catch(e => console.error(e)) seems idiomatic). Since I don't think either side of the argument is fully convincing, I'll leave it as an option, defaulting to just ['then'].

michaelfig commented 3 months ago

@dckc, consider pulling the latest https://github.com/endojs/endo/blob/mfig-o/packages/o/index.js