Skip to content

Fix stalled connections during session-key negotiation - #708

Open
nathansimony wants to merge 1 commit into
codetheweb:masterfrom
nathansimony:bugfix-handshake-reconnect
Open

nathansimony wants to merge 1 commit into
codetheweb:masterfrom
nathansimony:bugfix-handshake-reconnect

Conversation

@nathansimony

Copy link
Copy Markdown

Problem

When a protocol 3.4/3.5 device closes its TCP connection before session-key negotiation completes, connect() can remain pending forever. The socket close handler calls disconnect(), which returns early while _connected is false, leaving connectPromise cached. Subsequent callers receive the same unresolved promise.

The socket timeout is also removed on TCP connection, before authentication completes, so a silent handshake has no deadline. A caller awaiting connect() before scheduling its next retry can consequently stop retrying indefinitely.

Change

  • Keep the existing _connectTimeout as a deadline for the complete connection attempt, including session-key negotiation; clear it on success or cleanup.
  • Reject and release pending attempts on socket closure, explicit cancellation, socket/setup errors, or failed handshake authentication, and destroy their sockets.
  • Bind callbacks to their originating socket so a delayed event from an old attempt cannot tear down a retry started by an error/disconnected listener.
  • Preserve disconnected notifications for connections that actually reached the connected state.

The public options and retry policy are unchanged. This does not add automatic retries to TuyAPI; it allows callers' existing retry logic to run.

Validation

  • Added 19 regression tests. Protocol 3.4 and 3.5 tests use local TCP peers that perform the session-key exchange over the wire, covering mid-handshake closure, silence, incoming traffic that must not extend the deadline, cancellation, socket errors, authentication errors, setup errors, and immediate retries from event listeners. There is also a synchronous socket-options failure test.
  • The initial 12-test regression set failed against the unmodified implementation and passed with this change.
  • Full suite: 58 tests pass on Node.js 20.20.2 and 24.15.0. npm run coverage also passes on 24.15.0.
  • XO passes for the changed files under Node.js 20, with the existing _packetHandler complexity warning and no errors. The repository's older lint dependencies fail to start under Node.js 24 (util.isDate), so lint used Node.js 20.
  • Tested the same patched source on a Raspberry Pi 4 running Node.js 24.21.0, with a real CREATE fan using protocol 3.5. A local proxy deliberately closed the first handshake, then separately withheld its response. In both cases the same TuyAPI instance rejected the failed attempt, reconnected on its second attempt, and successfully read the fan's actual status. No fan settings were changed by the validation.

The original production outage is consistent with this failure mode, but its initial handshake was not captured; the regression tests independently demonstrate the library defect.

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