search

crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 274 forks source link

Complete CI test for all call cancelation features #1377

Open Dodobibi opened 6 years ago

Dodobibi commented 6 years ago

The implementation of call cancelling #299 won't respect the specification of the WAMP protocol.

As specified, the dealer must return an error to the caller (it do nothing) And depending of the mode (kill, killnowait or skip), the dealer must intercept the error given by the callee, and pas it to the caller (kill mode), or ignore it (killnowait or skip)

oberstet commented 6 years ago

connected to #1382

ecorm commented 6 years ago

We are using a C++ client (CppWAMP) for our backend which does not support the INTERRUPT and therefore does not include the call_canceling feature in its HELLO.Details.roles.callee.features.

We are using a Javascript client (Thruway.js) on the web browser front-end which can and does send CANCEL messages.

We are noticing that whenever Crossbar receives a CANCEL message from a browser, it sends an INTERRUPT to our C++ backend, causing the latter to drop the connection due to receiving an unsupported message type. This is with the latest Crossbar release (v18.7.2).

The behavior we would expect from Crossbar in this situation is that if the callee does not support call_canceling, then Crossbar will not propagate the cancellation to the callee in the form of an INTERRUPT message. This is called "skip mode" in the spec: https://wamp-proto.org/static/rfc/draft-oberstet-hybi-crossbar-wamp.html#rfc.section.14.3.4

All of the RPCs in our application are essentially non-blocking, so there's no inherent need for us to support INTERRUPT messages in our C++ backend.

meejah commented 4 years ago

Is this still a bug? I see a unit-test that looks like it tests this case (and passes).

meejah commented 4 years ago

@ecorm can you re-test your case with a recent crossbar? From code inspection and unit-tests I believe it should work correctly.

ecorm commented 4 years ago

@meejah Unfortunately, I'm working on something else not WAMP-related and won't have the chance to re-test anytime soon. When I switch back to WAMP mode, I'll let you know (and hope I won't forget!).

ecorm commented 4 years ago

@meejah I forgot to mention that we worked around this problem by implementing INTERRUPT in CppWAMP.

oberstet commented 4 years ago

Renaming/--labeling test issue accordlingly: we do have CI tests, but we should check that those tests cover the core of the feature, which is defined here:

For example, eg the original issue actually was:

ecorm commented 2 years ago

The skip mode in processCancel doesn't return an ERROR message back to the caller. This block of code that returns an ERROR message is never executed for skip mode due to this if clause.

Here's an annotated TCP wire dump that shows nothing happens after a client sends a CANCEL message with skip mode.

callee->router: ....[64,1,{},"rpc"] // REGISTER
router->callee: ....[65,1,7870435264840351] // REGISTERED
caller->router: ....[48,1,{},"rpc"] // CALL
router->callee: ....[68,480,7870435264840351,
                     {"caller":7411552417520712,"caller_authid":"redacted","caller_authrole":"anonymous"}]
                     // INVOCATION
caller->router: ....[49,1,{"mode":"skip"}] // CANCEL
<caller hangs here waiting for ERROR message that never comes>

This bug is not blocking any of my work; I'm just reporting it as a courtesy. I don't know if this report belongs here as a comment, or as a new Issue. Please feel free to migrate.

ecorm commented 1 year ago

Just a note that the bug reported in my previous comment no longer occurs for me with Crossbar v23.1.2.

oberstet commented 1 year ago

@ecorm thanks for reporting back! I will still leave open this issue .. until someone finds time to go over the automated tests ..