fix: add connect timeout to prevent indefinite hang on silent packet drops#36
Merged
Conversation
…drops Client.connect() wrapped net.createConnection() without a timeout, so a host that silently drops SYN packets (inverter WiFi dongle offline, firewall blackhole) left connect() hanging until the OS-level TCP timeout (~75s+). Downstream consumers (e.g. the Homey app) saw device init hang with no error and no progress signal. Guard connect() with a setTimeout using the existing client timeout option (default 4s). On fire, destroy the socket and reject with a clear error. Emit debug events on connect start and success so consumers get progress signals before polling begins. Persistent data/close/error handlers now attach only after a successful connect, avoiding a latent double-error path that could have both rejected and triggered _failAllPending on pre-connect errors. Fixes #35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Client.connect()now rejects within the configuredtimeout(default 4s) when the host silently drops SYN packets, instead of hanging until the OS-level TCP timeout (~75s+).onDebugevents on connect start and success so consumers see progress before polling begins.data/close/errorhandlers attach only after successful connect, fixing a latent double-error binding.Fixes #35.
Details
createConnectionwas wrapped without any timeout. When an inverter's WiFi dongle goes offline or a firewall blackholes port 8899, SYN packets are dropped silently and the socket neither connects nor errors for over a minute. Downstream (Homey app) reports showedInverter.connect()hanging with no debug signal, and Homey's init-timeout then killed and restarted the app in a loop.Fix reuses the existing
timeoutoption via asetTimeoutguard that destroys the socket and rejects with a clear message likeconnect timeout after 4000ms to 10.29.0.197:8899.Test plan
connect()rejects within the configured timeout when pointed at192.0.2.1:1(TEST-NET-1 blackhole).'connect'can fire before server's'connection'event is processed) by waiting for the server-side socket before asserting.npm run script:snapshot— connect/poll cycle still works end-to-end.