-
Notifications
You must be signed in to change notification settings - Fork 1
Fix WebSocket synchronization and reliability #2
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
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e212c5a
Wait for WebSocket handshake before reconnect reset
deleteLater dd15833
Stop reconnecting after WebSocket rejection
deleteLater cd71512
Return synchronization result details for unchanged data
deleteLater 9cec5ba
Make WebSocket shutdown completion reliable
deleteLater 0db3d64
Refactor WebSocket data synchronization into data_sync
deleteLater 9da4fc0
Return UNCHANGED for non data-sync messages
deleteLater bd7388c
Remove unnecessary note
deleteLater c90ff57
Ignore invalid WebSocket messages without disrupting synchronization
deleteLater 2a5833e
Harden shutdown cleanup and retain failed close results
deleteLater File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "timeout" | ||
| require "websocket-client-simple" | ||
|
|
||
| module FeatBit | ||
| # websocket-client-simple has no abort API. Keep its internal cleanup here, | ||
| # after the connector has exited and SDK callbacks have drained. | ||
| class ClosableWebSocketClient < WebSocket::Client::Simple::Client | ||
| CLOSE_TIMEOUT = 1.0 | ||
|
|
||
| def close(drain: false) | ||
| # The library also calls close from its own I/O error callbacks. Only the | ||
| # SDK owner may drain/kill the reader, after application callbacks return. | ||
| unless drain | ||
| @closed = true | ||
| return true | ||
| end | ||
|
|
||
| begin | ||
| Timeout.timeout(CLOSE_TIMEOUT) { super() } | ||
| rescue IOError, SystemCallError, OpenSSL::SSL::SSLError, Timeout::Error | ||
| # A failed close handshake is harmless if the ensure cleanup succeeds. | ||
| ensure | ||
| @closed = true | ||
| begin | ||
| @socket&.close | ||
| @socket = nil | ||
| ensure | ||
| @thread&.kill | ||
| @thread&.join unless Thread.current.equal?(@thread) | ||
| end | ||
| end | ||
| true | ||
| end | ||
| end | ||
| end |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module FeatBit | ||
| class SynchronizationResult | ||
| attr_reader :valid, :changed | ||
|
|
||
| def initialize(valid:, changed:) | ||
| @valid = valid | ||
| @changed = changed | ||
| freeze | ||
| end | ||
|
|
||
| def valid? = valid | ||
| def changed? = changed | ||
|
|
||
| INVALID = new(valid: false, changed: false) | ||
| UNCHANGED = new(valid: true, changed: false) | ||
| CHANGED = new(valid: true, changed: true) | ||
|
|
||
| def self.valid(changed:) | ||
| changed ? CHANGED : UNCHANGED | ||
| end | ||
| end | ||
| end |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require_relative "../status" | ||
|
|
||
| module FeatBit | ||
| class WebSocketClosePolicy | ||
| SERVER_REJECTED_CODE = 4003 | ||
|
|
||
| def initialize(status_provider) | ||
| @status_provider = status_provider | ||
| @mutex = Mutex.new | ||
| @rejected = false | ||
| end | ||
|
|
||
| def close_frame?(event) | ||
| event.respond_to?(:type) && event.type.to_sym == :close | ||
| rescue StandardError | ||
| false | ||
| end | ||
|
|
||
| def reject?(event) | ||
| return false unless close_code(event) == SERVER_REJECTED_CODE | ||
|
|
||
| @mutex.synchronize { @rejected = true } | ||
| reason = close_reason(event) | ||
| message = "WebSocket connection rejected by server (4003)" | ||
| message = "#{message}: #{reason}" unless reason.empty? | ||
| @status_provider.update(Status::FAILED, message: message) | ||
| true | ||
| end | ||
|
|
||
| def rejected? | ||
| @mutex.synchronize { @rejected } | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def close_code(event) | ||
| return event.code.to_i if event.respond_to?(:code) && event.code | ||
| return event[:code].to_i if event.is_a?(Hash) && event[:code] | ||
|
|
||
| event["code"].to_i if event.is_a?(Hash) && event["code"] | ||
| end | ||
|
|
||
| def close_reason(event) | ||
| value = if event.respond_to?(:reason) && event.reason | ||
| event.reason | ||
| elsif event.respond_to?(:data) && event.data | ||
| event.data | ||
| end | ||
| value.to_s | ||
| end | ||
| end | ||
| end |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "timeout" | ||
|
|
||
| module FeatBit | ||
| class WebSocketConnectionAttempt | ||
| attr_reader :socket | ||
|
|
||
| def initialize(timeout:, clock:, lifecycle:, closer:) | ||
| @deadline = clock.call + timeout | ||
| @clock = clock | ||
| @mutex = Mutex.new | ||
| @condition = ConditionVariable.new | ||
| @result = nil | ||
| @lifecycle = lifecycle | ||
| @closer = closer | ||
| @finished = false | ||
| end | ||
|
|
||
| def connect(connector, url, headers) | ||
| @connector_thread = Thread.new do | ||
| configured = false | ||
| @socket = connector.call(url, headers) do |connected_socket| | ||
| @socket = connected_socket | ||
| yield connected_socket | ||
| configured = true | ||
| end | ||
| yield @socket unless configured | ||
| rescue StandardError => e | ||
| @connect_error = e | ||
| end | ||
| until @connector_thread.join(0.01) | ||
| break if @lifecycle.stopped? || @clock.call >= @deadline | ||
| end | ||
| return @socket if @lifecycle.stopped? | ||
|
|
||
| raise Timeout::Error, "WebSocket connection timed out" if @connector_thread.alive? | ||
| raise @connect_error if @connect_error | ||
|
|
||
| @socket | ||
| end | ||
|
|
||
| def cleanup | ||
| # Cancellation is confined to the owned connector, never an application | ||
| # callback. Joining before close prevents a late connector creating a socket. | ||
| @connector_thread&.kill if @connector_thread&.alive? | ||
| @connector_thread&.join | ||
| @closer.call(@socket) | ||
| end | ||
|
|
||
| def signal(result) | ||
| @mutex.synchronize do | ||
| @finished = true unless result == :opened | ||
| return if @result | ||
|
|
||
| @result = result | ||
| @condition.broadcast | ||
| end | ||
| end | ||
|
|
||
| def wait(stopped:) | ||
| @mutex.synchronize do | ||
| until @result || stopped.call | ||
| remaining = @deadline - @clock.call | ||
| return :timeout unless remaining.positive? | ||
|
|
||
| @condition.wait(@mutex, [remaining, 0.05].min) | ||
| end | ||
| stopped.call ? :stopped : @result | ||
| end | ||
| end | ||
|
|
||
| def monitor(stopped:, ping:, interval:) | ||
| next_ping = @clock.call + interval | ||
| until stopped.call || @mutex.synchronize { @finished } || socket_closed? | ||
| if @clock.call >= next_ping | ||
| ping.call(@socket) | ||
| next_ping = @clock.call + interval | ||
| end | ||
| sleep(0.05) | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def socket_closed? | ||
| return @socket.closed? if @socket.respond_to?(:closed?) | ||
| return !@socket.open? if @socket.respond_to?(:open?) | ||
|
|
||
| false | ||
| rescue StandardError | ||
| true | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.