Fix/ws path normalisation#44
Merged
Merged
Conversation
Quinn review: if the server's route ever changes it is now findable and editable in one place. No behavior change. Signed-off-by: Blasius Patrick <blasius.patrick@gmail.com>
Before this change, pinger.Start() and d.Run() were both started before the dispatcher's ReadLoop had acquired the connection. The first pinger tick could fire before the dispatcher was inside its read select, meaning the server's pong response arrived as an "unsolicited pong" (handlePong: no active ping) and triggered notifyError. Fix: run d.Run in a goroutine and block for one 50ms read window before starting the pinger. The server sends hello as the first frame after connect, so one Read window is sufficient on any real network. The pinger's onPing closure already handles the conn-close race gracefully, so this is purely about eliminating the spurious notifyError call on the first pong. Signed-off-by: Blasius Patrick <blasius.patrick@gmail.com>
The handlePong case was calling notifyError() for every pong that arrived with no in-flight ping — an expected race condition, not a real error. The old comment said "log and continue" but notifyError fires OnError which prints a goroutine dump to stderr AND writes an audit entry, making it look like a fatal dispatch failure. Fix: remove the notifyError call entirely. An unsolicited pong means the node is fine (got a pong from the server). Just return silently. Also revert the previous attempt (goroutine + 50ms wait in reconnect.go) — that added complexity without solving the race because the pinger fires on its own interval, not immediately, so by the time it ticks the 50ms window has long passed. Signed-off-by: Blasius Patrick <blasius.patrick@gmail.com>
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.
No description provided.