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

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

Open
erights opened this issue Mar 26, 2024 · 4 comments
Assignees

Comments

@erights
Copy link
Member

erights commented Mar 26, 2024

const pmethods = harden(['then', 'catch', 'finally']);

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
Copy link
Member

dckc commented Mar 26, 2024

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
Copy link
Member Author

erights commented Mar 26, 2024

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
Copy link
Member

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
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants