diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 9cfcd6cc..127e9d31 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -32,8 +32,9 @@ jobs: run: | mkdir junit bundle exec parallel_rspec --prefix-output-with-test-env-number --first-is-1 -- spec/${{ matrix.type }} - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: + name: test-results-ruby-${{ matrix.ruby }}-${{ matrix.protocol }}-${{ matrix.type }} path: | junit/ coverage/ @@ -45,7 +46,7 @@ jobs: server-url: 'https://test-observability.herokuapp.com' server-auth: ${{ secrets.TEST_OBSERVABILITY_SERVER_AUTH_KEY }} path: 'junit/' - - uses: coverallsapp/github-action@1.1.3 + - uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.GITHUB_TOKEN }} flag-name: ruby-${{ matrix.ruby }}-${{ matrix.protocol }}-${{ matrix.type }} @@ -55,7 +56,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Coveralls Finished - uses: coverallsapp/github-action@1.1.3 + uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.github_token }} parallel-finished: true diff --git a/ably.gemspec b/ably.gemspec index d301cc80..f94291d1 100644 --- a/ably.gemspec +++ b/ably.gemspec @@ -25,7 +25,8 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency 'faraday-typhoeus', '~> 0.2.0' spec.add_runtime_dependency 'typhoeus', '~> 1.4' spec.add_runtime_dependency 'json' - spec.add_runtime_dependency 'websocket-driver', '~> 0.7' + # We disallow minor version updates, because this gem has introduced breaking API changes in minor releases before (which it's within its rights to do, given it's pre-v1). If you want to allow a new minor version, bump here and run the tests. + spec.add_runtime_dependency 'websocket-driver', '~> 0.8.0' spec.add_runtime_dependency 'msgpack', '>= 1.3.0' spec.add_runtime_dependency 'addressable', '>= 2.0.0' diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 6295ef09..45e6abe5 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -690,16 +690,44 @@ def disconnected_from_connected_state? end def use_fallback_if_disconnected? - second_reconnect_attempt_for(:disconnected, 1) + unless second_reconnect_attempt_for(:disconnected, 1) + return false + end + + does_error_necessitate_fallback(reason_for_last_time_in(:disconnected)) end def use_fallback_if_suspended? - second_reconnect_attempt_for(:suspended, 2) # on first suspended state use default Ably host again + unless second_reconnect_attempt_for(:suspended, 2) # on first suspended state use default Ably host again + return false + end + + does_error_necessitate_fallback(reason_for_last_time_in(:suspended)) end def second_reconnect_attempt_for(state, first_attempt_count) previous_state == state && manager.retry_count_for_state(state) >= first_attempt_count end + + # Provides a partial implementation of RTN17f's logic for whether an error necessitates a fallback host. + def does_error_necessitate_fallback(error) + return false unless error + + # For now we just explicitly exclude token errors. TODO: implement properly in https://github.com/ably/ably-ruby/issues/444 + + if error.respond_to?(:status_code) && error.status_code == 401 && error.respond_to?(:code) && Ably::Exceptions::TOKEN_EXPIRED_CODE.include?(error.code) + return false + end + + true + end + + # Returns the error associated with the last state change to the given state (e.g. :disconnected). + def reason_for_last_time_in(state) + history_item = state_history.reverse.find do |history_item| + history_item.fetch(:state) == state + end.fetch(:metadata).reason + end end end end diff --git a/lib/ably/realtime/connection/websocket_transport.rb b/lib/ably/realtime/connection/websocket_transport.rb index 1d348c9e..fc50dd19 100644 --- a/lib/ably/realtime/connection/websocket_transport.rb +++ b/lib/ably/realtime/connection/websocket_transport.rb @@ -257,7 +257,7 @@ def parse_event_data(data) when :json JSON.parse(data) when :msgpack - MessagePack.unpack(data.pack('C*')) + MessagePack.unpack(data) else client.logger.fatal { "WebsocketTransport: Unsupported Protocol Message format #{client.protocol}" } data diff --git a/spec/acceptance/realtime/client_spec.rb b/spec/acceptance/realtime/client_spec.rb index b392d118..131ab8f4 100644 --- a/spec/acceptance/realtime/client_spec.rb +++ b/spec/acceptance/realtime/client_spec.rb @@ -34,7 +34,7 @@ client.connection.once(:failed) do expect(custom_logger_object.logs.find do |severity, message| next unless %w(fatal error).include?(severity.to_s) - message.match(%r{https://help.ably.io/error/40400}) + message.match(%r{https://help.ably.io/error/40101}) end).to_not be_nil stop_reactor end diff --git a/spec/acceptance/realtime/connection_failures_spec.rb b/spec/acceptance/realtime/connection_failures_spec.rb index 6afb7616..64a1225b 100644 --- a/spec/acceptance/realtime/connection_failures_spec.rb +++ b/spec/acceptance/realtime/connection_failures_spec.rb @@ -26,13 +26,13 @@ context 'with invalid app part of the key' do let(:invalid_key) { 'not_an_app.invalid_key_name:invalid_key_value' } - it 'enters the failed state and returns a not found error' do + it 'enters the failed state and returns an invalid credentials error' do connection.on(:failed) do |connection_state_change| error = connection_state_change.reason expect(connection.state).to eq(:failed) # TODO: Check error type is an InvalidToken exception - expect(error.status).to eq(404) - expect(error.code).to eq(40400) # not found + expect(error.status).to eq(401) + expect(error.code).to eq(40101) # invalid credentials stop_reactor end end @@ -46,7 +46,7 @@ error = connection_state_change.reason expect(connection.state).to eq(:failed) # TODO: Check error type is a TokenNotFound exception - expect(error.status).to eq(401) + expect(error.status).to eq(404) expect(error.code).to eq(40400) # not found stop_reactor end @@ -1396,10 +1396,10 @@ def kill_connection_transport_and_prevent_valid_resume channel = client.channels.get("foo") channel.attach do connection.once(:failed) do |state_change| - expect(state_change.reason.code).to eql(40400) - expect(connection.error_reason.code).to eql(40400) + expect(state_change.reason.code).to eql(40101) + expect(connection.error_reason.code).to eql(40101) expect(channel).to be_failed - expect(channel.error_reason.code).to eql(40400) + expect(channel.error_reason.code).to eql(40101) stop_reactor end diff --git a/spec/acceptance/rest/base_spec.rb b/spec/acceptance/rest/base_spec.rb index 0e046458..49fd2d5c 100644 --- a/spec/acceptance/rest/base_spec.rb +++ b/spec/acceptance/rest/base_spec.rb @@ -72,13 +72,12 @@ describe 'failed requests' do context 'due to invalid Auth' do - it 'should raise an InvalidRequest exception with a valid error message and code' do + it 'should raise an UnauthorizedRequest exception with a valid error message and code' do invalid_client = Ably::Rest::Client.new(key: 'appid.keyuid:keysecret', environment: environment) expect { invalid_client.channel('test').publish('foo', 'choo') }.to raise_error do |error| - expect(error).to be_a(Ably::Exceptions::ResourceMissing) - expect(error.message).to match(/No application found/) - expect(error.code).to eql(40400) - expect(error.status).to eql(404) + expect(error).to be_a(Ably::Exceptions::UnauthorizedRequest) + expect(error.code).to eql(40101) + expect(error.status).to eql(401) end end end diff --git a/spec/acceptance/rest/client_spec.rb b/spec/acceptance/rest/client_spec.rb index e1619c6e..219ba75b 100644 --- a/spec/acceptance/rest/client_spec.rb +++ b/spec/acceptance/rest/client_spec.rb @@ -33,9 +33,9 @@ def encode64(text) it 'logs an entry with a help href url matching the code #TI5' do begin client.channels.get('foo').publish('test') - raise 'Expected Ably::Exceptions::ResourceMissing' - rescue Ably::Exceptions::ResourceMissing => err - expect err.to_s.match(%r{https://help.ably.io/error/40400}) + raise 'Expected Ably::Exceptions::UnauthorizedRequest' + rescue Ably::Exceptions::UnauthorizedRequest => err + expect err.to_s.match(%r{https://help.ably.io/error/40101}) end end end