karatelabs / karate

Test Automation Made Simple
https://karatelabs.github.io/karate
MIT License
8.09k stars 1.94k forks source link

WebSocketClient should allow for listening multiple times #2003

Closed jon-armen closed 1 year ago

jon-armen commented 2 years ago

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' }
ptrthomas commented 2 years ago

@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

jon-armen commented 2 years ago

@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?

ptrthomas commented 2 years ago

@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

jon-armen commented 2 years ago

@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 commented 2 years ago

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

ptrthomas commented 2 years ago

@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

jon-armen commented 2 years ago

@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

ptrthomas commented 2 years ago

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

jon-armen commented 2 years ago

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

jon-armen commented 2 years ago

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

ptrthomas commented 2 years ago

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

prnbtr09 commented 2 years ago

@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

jon-armen commented 2 years ago

@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));

ptrthomas commented 2 years ago

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

jon-armen commented 2 years ago

@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.

ptrthomas commented 2 years ago

@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

prnbtr09 commented 2 years ago

@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

ptrthomas commented 2 years ago

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

ptrthomas commented 2 years ago

@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

prnbtr09 commented 2 years ago

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

jon-armen commented 2 years ago

@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.

ptrthomas commented 2 years ago

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

ptrthomas commented 1 year ago

1.3.0 released