karate icon indicating copy to clipboard operation
karate copied to clipboard

WebSocketClient should allow for listening multiple times

Open jon-armen opened this issue 2 years ago • 22 comments

Currently, once the WebSocketClient produced by karate.webSocket(url, handlerDelegate); has responded to it's listen method, subsequent calls to listen will always return the first message returned. This is not helpful, particularly when you may change the handler via setTextHandler after the first listen.

Here's the rough scenario I'm running into.

Our WebSocket API requires authentication sent thru the first frame before we will receive messages. That authentication may take a few seconds. Currently, as soon as I send the auth frame in Karate, the rest of the suite executes. Since the authentication did not complete, we don't receive any of the expected messages on from the socket connection. Our objective would be to send the auth frame, then wait for a response. Once we have the expected auth response, continue with the rest of our test and then listen for the next expected web socket.

Our code roughly follows this pattern:

var msgHandlerDelegate = function(msg) {
  var jsonMsg = JSON.parse(msg);
  if (jsonMessage.eventType == 'ExpectedEvent') {
    return true;
  }
  return false;
};

var authHandlerDelegate = function(msg) {
  var jsonMsg = JSON.parse(msg);
  if (jsonMsg.statusCode == 200 && jsonMsg.body == "OK") {
    return true;
  }
  return false;
};

var webSocketClient = karate.webSocket(websocketUrl, authHandlerDelegate);

var authPayload = { "message": "auth","data": { "auth_token": auth_token } };
webSocketClient.send(JSON.stringify(authPayload));
var response = webSocketClient.listen(20000);

expect response != null;

webSocketClient.setTextHandler(karate.toJava(msgHandlerDelegate));

// Perform remainder of test

var webSocketMessage = webSocketClient.listen(20000);
expect webSocketMessage == { eventType: 'ExpectedEvent' }

jon-armen avatar May 10 '22 02:05 jon-armen

@jon-armen sounds good and thanks for the PR. do you think you can add a test for your PR say over here or anywhere else you choose: https://github.com/karatelabs/karate/blob/master/karate-demo/src/test/java/demo/websocket/websocket.feature

ptrthomas avatar May 10 '22 02:05 ptrthomas

@ptrthomas , absolutely. I'll see if I can get that for you shortly. Would you happen to have an ETA when this change could be released?

jon-armen avatar May 10 '22 02:05 jon-armen

@jon-armen timing is a little awkward since I just released 1.20 final after 6 months :| but we can plan a 1.3.0.RC1 or something. we will need to wait a week or two to accumulate more emergency fixes if any.

I've heard that jitpack can give you a public maven artifact if you are in a hurry: https://twitter.com/maxandersen/status/1277116280542281729

ptrthomas avatar May 10 '22 03:05 ptrthomas

@ptrthomas , understandable. I am definitely excited to see the progress you have been making on this project, and the 1.20 release is another great milestone.

jon-armen avatar May 10 '22 03:05 jon-armen

@ptrthomas I've updated the samples to include tests. Verified before these samples would fail, and now they pass.

jon-armen avatar May 10 '22 03:05 jon-armen

@jon-armen the ci build broke, most likely because the demo websocket server is not good with concurrent clients.

but needs investigation, for now I've commented out the test steps that break: https://github.com/karatelabs/karate/commit/73be0067698e0c775aa7748471b0c06775cd9155

ptrthomas avatar May 10 '22 04:05 ptrthomas

@ptrthomas , I'm not familiar with your build system, but is it possible it's using jar's from Karate 1.2.0 rather than from Core? I noticed when I was running it locally, I had to overwrite my package for Karate 1.2.0 to get my tests to pass.

Noting from the build log:

HTML report: (paste into browser to view) | Karate version: 1.2.0 file:///home/runner/work/karate/karate/karate-demo/target/karate-reports/karate-summary.html

jon-armen avatar May 10 '22 04:05 jon-armen

@jon-armen no, you can replicate locally by running mvn test even in the karate-demo project itself

ptrthomas avatar May 10 '22 05:05 ptrthomas

@ptrthomas , got it. I can also repo it locally.

jon-armen avatar May 10 '22 05:05 jon-armen

@ptrthomas , thank you for your responsiveness getting these changes incorporated. much appreciated.

jon-armen avatar May 10 '22 05:05 jon-armen

@jon-armen 1.2.1.RC2 should be in maven central now, let me know if it works for you

ptrthomas avatar Jun 07 '22 04:06 ptrthomas

@ptrthomas These changes are not working in my case, I am trying to change handler, it is giving me null for second listen. getting following error: [*** execute ***] invocation failed: Multi threaded access requested by thread

prnbtr09 avatar Jun 27 '22 13:06 prnbtr09

@prnbtr09 , I had this issue as well when I was working on this. I found that when replacing the handler, I often had to use karate.toJava()

webSocketClient.setTextHandler(karate.toJava(msgHandlerDelegate));

jon-armen avatar Jun 27 '22 17:06 jon-armen

@prnbtr09 kindly follow this process: https://github.com/karatelabs/karate/wiki/How-to-Submit-an-Issue

ptrthomas avatar Jun 27 '22 18:06 ptrthomas

@ptrthomas , I haven't had the chance to fully test this on my project yet. When upgrading to 1.2.0, we have a series of failures due to sequencing of calling GIVEN path = '' and other call read functions. I believe this is more of an issue with our code that it is the Karate framework. I'm hoping that we can fix up our project within the next few weeks and really be able to give this a test.

jon-armen avatar Jun 27 '22 19:06 jon-armen

@prnbtr09 @jon-armen okay I just remembered that upgrading graal introduced some problems on the websockets front which I have to look into. community help will be appreciated. here are the details: https://github.com/karatelabs/karate/commit/617348ca1742eb2e6a9cde9bcf8e035ee475d7af

we have a pending refactoring which is tricky, any help is welcome: https://github.com/karatelabs/karate/issues/1883

as a workaround, consider rolling your own websocket client, refer: https://stackoverflow.com/a/69299321/143475

ptrthomas avatar Jun 27 '22 19:06 ptrthomas

@ptrthomas using the below stackoverflow link, I am unable to set handler again and unable to recive new message: https://stackoverflow.com/a/69299321/143475

prnbtr09 avatar Jun 28 '22 08:06 prnbtr09

@prnbtr09 I've nothing to add to my comment above

ptrthomas avatar Jun 28 '22 10:06 ptrthomas

@jon-armen with the graal-js factoring we had to change the design of websocket support a little bit. this is a breaking change in the next version. this diff view should explain it:

image

this is now consistent with how the listen keyword was designed, when we first started running into the graal-js limitation. long story: #2081

now things look good in develop - it would be great if you can review and see if the multiple messages use case works fine

ptrthomas avatar Aug 04 '22 12:08 ptrthomas

@ptrthomas Do we have provided the fix for Karate Websocket for listening multiple messages

prnbtr09 avatar Aug 10 '22 10:08 prnbtr09

@ptrthomas , Apologies for not getting back sooner, No worries about a breaking change on the implementation. I'll try to test it out in the next few days.

jon-armen avatar Aug 10 '22 11:08 jon-armen

@prnbtr09 yes, see: https://github.com/karatelabs/karate/wiki/1.3.0-Upgrade-Guide

ptrthomas avatar Aug 10 '22 11:08 ptrthomas

1.3.0 released

ptrthomas avatar Nov 02 '22 17:11 ptrthomas