capnproto / node-capnp

Cap'n Proto bindings for Node.js
BSD 2-Clause "Simplified" License
258 stars 35 forks source link

Cannot call interface methods named 'close' #33

Open mologie opened 7 years ago

mologie commented 7 years ago

The following will not work due to close() being replaced by the node-capnp implementation:

interface DirectionControllableShutter {
    open @0 ();
    close @1 ();
    stop @2 ();
}
this.controller.getDirectionControllableShutter(id)
    .then((result) => result.object.close())
    .then((result) => { /* ... */ })

I'm aware that this is by design, but would raise an issue nevertheless because the capnp compiler allows methods to be named 'close'. As such, I'd expect them to be callable, even if it requires a language-specific workaround (which might also be required when naming a method after a language's reserved word.)

If you have a suggestion for a clean and generic workaround I'd be happy to implement it and submit a pull request. Ideally, this should be done without introducing an edge-case.

A quick and simple solution would be to detect clashes and resolve them by appending an underscore to the method name, so that the above would say .then((result) => result.object.close_()). I think that this is bad, because an edge-case is introduced which must be documented and communicated to the user.

Cheers and a happy new year, Oliver

kentonv commented 7 years ago

I'm not sure how we could solve this without renaming the RPC method to close_() or the like. It's important that the close() method on a capability has the same behavior regardless of the capability's interface, so that code which operates on general capabilities can successfully call it. So, unless we want to break API compatibility, it seems like the only option if there is a conflicting RPC method is to rename the RPC method.

Out of curiosity, why does your interface have a close() method? Usually, with Cap'n Proto, "close" behavior should be implemented in the object's destructor, not as a method.

mologie commented 7 years ago

The above snippet is from my home automation software, where close quite literally closes a physical shutter (of a window.) No memory management concepts have been violated during development of said software :-).

I ended up renaming the RPC method as workaround, but can imagine the issue to become a real problem in practice when trying to access a remote service whose protocol/interfaces one does not control.

kentonv commented 7 years ago

Haha, fair enough.

Generally our solution to name collisions has been to append an underscore, so I think that's the answer here unless someone has an alternative proposal.