Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network connection interruption not detected #276

Closed
Javierd opened this issue Feb 10, 2023 · 16 comments · Fixed by #400
Closed

Network connection interruption not detected #276

Javierd opened this issue Feb 10, 2023 · 16 comments · Fixed by #400

Comments

@Javierd
Copy link

Javierd commented Feb 10, 2023

Describe the bug
When the client is connected to a server and the client network fails (e.g., the data and wifi are disabled) the client never notices the problem, and when trying to execute a RPC call, the call never returns, and doesn't throw any errors neither.

To Reproduce
Steps to reproduce the behavior:

  1. Connect to a remote server
  2. Disable the client's network connections
  3. Note that the client never notices the problem
  4. Try to execute a RPC call
  5. Note that the client never notices the problem

Expected behavior
The component should note that the connection is lost, it should retry to connect and, if it can't, it should call the onclose callback.

Environment (please complete the following information):

  • OS: Linux
  • Browser and version: Chrome 109.0.5414.119
  • Version of Wampy.js library: 7.0.2
  • Wampcc master branch
@KSDaemon
Copy link
Owner

Hi @Javierd Hm... Interesting question... From one side — that seems to be the problem on websocket connection side.... But in the end — it doesn't matter for end user. Need to investigate this...

@Javierd
Copy link
Author

Javierd commented Feb 16, 2023

Sure, although it's related to a fail on the websocket, the wamp client should get notified and try to reconnect.
It would also be useful being able to programatically check if the client is connected, with some kind of isConnected() method.

@KSDaemon
Copy link
Owner

Well, if underlying websocket object reports the failure — wampy of course handles that: it reconnects if configured and so on. So as I see the problem is that websocket is not reporting the error/disconnection...

@KSDaemon
Copy link
Owner

Rgd isConnected() — well. I don't have a need for this all these years :) that's why it is not implemented yet.
Because for most cases you can understand the current state with provided callbacks without need to poll the state like onReconnect() or others:

  • onClose. Default value: null. Callback function. Fired on closing connection to wamp server.
  • onError. Default value: null. Callback function. Fired on error in websocket communication.
  • onReconnect. Default value: null. Callback function. Fired every time on reconnection attempt.
  • onReconnectSuccess. Default value: null. Callback function. Fired every time when reconnection succeeded.

@Javierd
Copy link
Author

Javierd commented Feb 20, 2023

Yeah, I currently detect the current state using those callbacks, but I though it would be handy to have that isConnected method. Anyway, that is not importart.
Do you have any idea on why the web socket could be missing to notify that the connection has ended? For me, and I would say for every production usage, this feature is pretty important

@KSDaemon
Copy link
Owner

KSDaemon commented Feb 20, 2023

Well, I haven't got there yet, sorry. But I think the answer is somewhere deeper in the underlying network stack implementation. And afaik the only way to detect it — is a kind of keepalive/ping thing. In that case, when the browser/node tries to send a packet — it will face the problem from the network stack so all callback events will be fired as expected.

Look for example at this SO: https://stackoverflow.com/questions/26971026/handling-connection-loss-with-websockets

Oh, btw, what WAMP server/Router do you use? Look at docs, there might be an option to set the websocket ping/pong poll for connection. (of course that doesn't solve the problem for the client side, but still might be helpfull)

Unfortunately, that low-level feature (ping/pong frames) is not available in the browser websocket object :(
So in general, this can be implemented only by sending some legitimate WAMP message (e.g. Sending an empty websocket message might not work if the router expects to get a fulfilled message)

@KSDaemon
Copy link
Owner

So my advice for a quickfix is to implement a kind of super lightweight dummy RPC (like status/keepalive/check/whatever) on the backend side and call it from the client periodically.

But we may try to find a more universal solution...

@Javierd
Copy link
Author

Javierd commented Feb 21, 2023

I'm currently using wampcc as the wamp server, and in fact I was planning on adding that kind of heartbeat RPC call. However, the problem is that once the client is disconnected, if I try to call a wamp RPC function, it never returns, so it doesn't event notice that it's disconnected in this way

@Javierd
Copy link
Author

Javierd commented Aug 10, 2023

Hi!
Any updates on this topic? I'm still experiencing the same issue and it seems that even when setting the timeout option when performing a RPC call, if the network is disconnected the promise never finishes. However, on Chrome's network tab I can see that the client seems to detect the issue:
image

@KSDaemon
Copy link
Owner

Hi @Javierd Unfortunately haven't had much time to get to look yet, but I think I have an idea where the problem might be. I'll try to take a look at it soon....

@KSDaemon
Copy link
Owner

@Javierd can you try Wampy from the dev branch in your env? Hope it will resolve your issue. It will be very helpful if you can check that!

@Javierd
Copy link
Author

Javierd commented Aug 11, 2023

I've been trying it but it doesn't seem to work. In case it helps you, my use case is the following one:
I need to detect when the connection is not working in case the client has been suddenly disconnected from the network, so I use a timer to make periodic calls to a simple RPC and wait for the response. I make the RPC heartbeal call every 5 seconds. If after 15 seconds I have not received a response, I call wamp_client.disconnect(), as I assume the connection is lost.
The problem is that, after calling disconnect(), every promise reminds running and they never finish.

Also, this problem may be related to the timeout option of wamp_client.call() method, which is not working neither in this scenario. I've solved it by using a new Promise with a timeout along with Promise.race

@KSDaemon KSDaemon reopened this Aug 11, 2023
@KSDaemon
Copy link
Owner

Okay. Thnx for additional details. I tuned some things more. Will try to test it (actually that requires a bit more time, then just coding :) )
It's strange a bit, cause promises should be rejected on websocket closing

    async _wsOnClose (event) {
        this._log('websocket disconnected. Info: ', event);

        await this._reject_ongoing_promises(new WebsocketError('Connection closed'));
         ......
    }

@KSDaemon
Copy link
Owner

Also, this problem may be related to the timeout option of wamp_client.call() method, which is not working neither in this scenario. I've solved it by using a new Promise with a timeout along with Promise.race

call timeout option must be processed on the router or callee side. Right now it is not processed on the wampy's side.

@KSDaemon
Copy link
Owner

@Javierd I have a next code:

const wampy = new Wampy('ws://127.0.0.1:8080/ws', options);

await wampy.connect();

const p1 = wampy.call('test.rpc.com');

p1.catch(function (reason) {
    console.log('catching:', reason);
});

await wampy.disconnect();

And p1.catch works as expected. Can you try once more the latest dev branch and check if this issue still happens?
If yes - can you provide a reproducible code snippet or sandbox example? If no - please close the issue :)

@KSDaemon KSDaemon reopened this Jan 11, 2024
@KSDaemon
Copy link
Owner

I'll close it for now. Feel free to reopen in case of experiencing this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants