Skip to content

Conversation

@he-is-harry
Copy link
Contributor

  • Added initial isValid(timeout) check which will return true if the client can send a request to the server and receive a reply within the timeout
    • timeout is given in seconds
    • The API's implementation can be updated once the ping protocol support is complete on the server side
  • Updated Connection.send so that it can accept a communicationTimeout option to timeout a request to the server and return a timeout error
  • Added unit and integration tests for isValid

- Added initial isValid(timeout) check which will return true if the
client can send a request to the server and receive a reply within
the timeout
    - timeout is given in seconds
- Updated Connection.send so that it can accept a communicationTimeout
option to timeout a request to the server and return a timeout error
- Added unit and integration tests for isValid
@he-is-harry
Copy link
Contributor Author

he-is-harry commented Apr 25, 2025

Here is a rough draft for the reconnect on timeout change: he-is-harry@bcdd7e0.

TODOs:

  • Testing, the change is a rough draft so little cases were tested, so several more tests should be run to check that the queue always runs the newly enqueue tasks
    • Things to try: Multiple reconnects, affect of reconnect on transaction, pending tasks should currently receive an error in callback after timeout in current task
  • Investigate the connection properties that should be used for reconnect (maybe _redirectHost / _redirectPort is better?)
  • Adding a connection readyState for the 'ready for reconnect' state after timeouts would be helpful
  • Currently reconnect does not support redirection, perhaps there should be an integration with ConnectionManager
    • The Client and ConnectionManager connect code could be moved into Connection as a refactoring change
  • The added Connection.reset function can be refactored to reduce duplicate code in Connection constructor
  • Packet count check can be removed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant