capnproto / go-capnp

Cap'n Proto library and code generator for Go
https://capnproto.org
Other
1.2k stars 108 forks source link

rpc.Conn doesn't support promise capabilities #2

Open zombiezen opened 9 years ago

zombiezen commented 9 years ago

From @zombiezen on August 6, 2015 21:1

The CapDescriptor struct has a senderPromise field which rpc.Conn does not support, along with the corresponding Resolve message. The Go implementation does not emit this descriptor type, so it is only a concern if interoperating with another implementation that does.

Copied from original issue: zombiezen/go-capnproto#2

zenhack commented 8 years ago

As you suspected, (I think) this is causing problems trying to do stuff with WebSession. I'm getting these errors on the console for irc-idler:

2016/05/23 17:44:18 rpc: unknown capability type senderPromise
sandstorm/supervisor.c++:1893: error: exception = capnp/rpc.c++:2643: disconnected: RpcSystem was destroyed.
stack: 0x44f1f0 0x4616c0 0x46de50 0x5e2fa0 0x5d0560 0x5cfe80
2016/05/23 17:44:18 rpc: aborted by remote: Peer did not implement required RPC message type.; (uint)message.which() = 2
zombiezen commented 8 years ago

So a partial workaround is to use an error client in populateMessageCapTable. Unfortunately, this will break down if you need to actually use the capability (which I can't recall whether you need it or not). Does that work?

zombiezen commented 8 years ago

The actual fix is a larger semantic change and I don't have the time to work on it right now.

zenhack commented 8 years ago

My guess is that's not going to cut it, but I should do some more digging before I let you quote me on that. It's not what I'm hitting right this second, but it looks like streaming HTTP responses involves a promise, so without them I'd have to buffer the whole thing and send when the connection is closed. Haven't looked at the websocket calls, but my guess is that's not going to go well either.

Any pointers about what needs to change for a proper fix?

zombiezen commented 8 years ago

Drat. It's been a while since I've looked at the spec, but generally:

question.go will help you the most, but it the edge cases are quite subtle. I'm not entirely sure what the API should look like if Go were to send these, but if we don't come up with an API, proper integration testing would be difficult (since we'd have to rely on creating them in the wire format).

kentonv commented 8 years ago

One easy way to work around this might be to treat senderPromise the same as senderHosted and ignore all Resolve messages. That will at least allow messages to be delivered to the promise. The down sides are:

kentonv commented 8 years ago

Correction: You shouldn't ignore Resolve but rather reply with unimplemented, reflecting the message back. I believe the C++ implementation will correctly drop the unused capability when it receives this.

zombiezen commented 8 years ago

Neat. The unimplemented logic should already be there, so maybe it is just a simple matter of hacking those cases to be treated identically. Thanks @kentonv!

zenhack commented 6 years ago

Looks like the v3 branch has regressed on this; recvCap doesn't have a case for senderPromise. Any reason not to implement the same workaround there as we did for the v2 branch, in the interm?

zombiezen commented 6 years ago

Same workaround could apply. That was the next thing I was going to tackle, but it's looking like I won't be able to get back to v3 for a while.

zenhack commented 2 years ago

I think this probably doesn't need to be on the v3 milestone; it's all internals, so shouldn't require breaking compatibility to address. Any objections to removing it?