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

Add timeout params to websocket client #346

Open
AlexHill opened this issue Dec 21, 2017 · 6 comments
Open

Add timeout params to websocket client #346

AlexHill opened this issue Dec 21, 2017 · 6 comments
Labels
good first issue A good issue for newcomers

Comments

@AlexHill
Copy link

Hello,

I have a number of client devices connecting as websocket clients to a server. To handle network interruptions, I reconnect to the WS server after each disconnection, with some backoff, etc.

Some of these devices are on unreliable networks, and I'm sometimes seeing websocket-client fail to time out and simply hang indefinitely. I think what's happening is that the actual socket connection is succeeding, but then the client is losing connectivity during the handshake. Without a keepalive on the TCP connection, or a timeout handler on the Netty channel, the deferred never succeeds or fails.

I'm using deferred/timeout! to work around this but I'm not sure what's happening under the hood - i.e. whether or this means that sockets/resources are still allocated and hanging around forever in the background.

Adding an IdleStateHandler to the Netty channel while the handshake is in progress might fix this.

Alex

@tendant
Copy link

tendant commented Feb 11, 2018

Observed the same issue. Thanks @AlexHill for pointing out the use of manifold.deferred/timeout!.

@shilder
Copy link
Contributor

shilder commented Oct 8, 2018

Related to #420

Don't know about timeouts, but handshake exceptions were ignored and left opened socket

@matthewdowney
Copy link

I'm working with a similar situation (lots of unreliable connections, restarts with some backoff, etc.). I'm wondering what the idiomatic way to enforce a timeout would be?

Like, if I open the connection with something like the following, before setting any handlers, am I still creating a situation where the aleph-netty-client-event-pool-<N>-<N> threads could end up permanently in a WAITING state?

(defn try-connect [uri timeout-ms]
  (let [pending-sock (http/websocket-client uri)]
    (-> pending-sock
        (d/timeout! timeout-ms ::timeout)
        (d/chain
          (fn [open-sock-or-timeout]
            (if (= open-sock-or-timeout ::timeout)
              (do
                ;; Make sure that if/when the socket does eventually open, it
                ;; gets closed
                (d/chain pending-sock (fn [open-sock] (s/close! open-sock)))
                nil)
              open-sock-or-timeout))))))

Then I'd do something along the lines of this to consume the connection:

(d/let-flow [sock (try-connect "wss://foo.bar/" 10000)]
  (if sock 
    (do
      (println "Connection open.")
      (d/chain 
        (s/consume (fn [msg] (println ">" msg)) sock)
        (fn [_] (println "Connection closed."))))
    (println "Connection timed out, retrying...")))

Is there a better way to do this?

@KingMob
Copy link
Collaborator

KingMob commented Sep 21, 2022

@AlexHill @tendant I know it's been a while. Is this still an issue? Did #420 fix the problem? If not, do either of you have some code you can share to demo the problem?

Alex, when you said "Adding an IdleStateHandler to the Netty channel while the handshake is in progress might fix this", did you try adding one with the pipeline-transform option? If so, did that fix it?

All that being said, it looks like Netty added more timeout options to the Websocket code in netty/netty#8856, so it might just be a matter of passing some more params to WebSocketClientHandshakerFactory/newHandshaker.

@KingMob KingMob added the good first issue A good issue for newcomers label Sep 21, 2022
@AlexHill
Copy link
Author

Thanks for taking a look!

I’m no longer in the job where I encountered this, but I believe making use of the new netty handshake is the right fix for this in 2022. 🙂

@KingMob
Copy link
Collaborator

KingMob commented Sep 21, 2022

Todo:

  • Create test demonstrating handshake hanging
  • Add forceCloseTimeoutMillis param to WebSocketClientHandshakerFactory/newHandshaker call, using defaults for the masking params. Does this fix the issue?
  • Alternatively, investigate using a higher-level handler in the pipeline, like WebSocketClientProtocolHandler. Would that allow us to stop maintaining uninteresting code, like handling handshakes and Pings? Or would it cause more problems than it's worth?

@KingMob KingMob changed the title websocket-client can hang indefinitely Add timeout params to websocket client Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good issue for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants