From 92fcb2be8e57536718131b8aada5ebe36a923a16 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 10 Jun 2025 08:51:26 -0300 Subject: [PATCH 1/6] Update version of upload-artifact action v3 no longer works. --- .github/workflows/check.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 9cfcd6cc..a3dd4e49 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/ From 1cc570b11f16d59766e6d9cbd1e26f86e70581e7 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 10 Jun 2025 09:06:17 -0300 Subject: [PATCH 2/6] Update Coveralls GitHub action To get rid of a GitHub warning about using `set-output`. --- .github/workflows/check.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index a3dd4e49..127e9d31 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -46,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 }} @@ -56,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 From 6c76d0624f71ea37c1a9f2880274c5a00bcfe19c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 11 Jun 2025 10:59:53 -0300 Subject: [PATCH 3/6] Fix decoding of MessagePack data websocket-driver 0.8 changed the format in which it emits binary data [1]: > Emit binary message as a string with Encoding::BINARY instead of an array > Add the option :binary_data_format to force the previous behaviour [1] https://github.com/faye/websocket-driver-ruby/blob/main/CHANGELOG.md#080--2025-05-25 --- ably.gemspec | 3 ++- lib/ably/realtime/connection/websocket_transport.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) 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/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 From 9db1057ddcebd35455d407c9fa0841d0a3d2e8a6 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 11 Jun 2025 11:16:38 -0300 Subject: [PATCH 4/6] Change expectation of error code for invalid app name This was changed in Realtime a while ago [1], and we've already had to update other SDKs' test suites to handle it. Also, remove an assertion of an error message, as in 72120de. [1] https://ably-real-time.slack.com/archives/CURL4U2FP/p1733420816469159 --- spec/acceptance/realtime/client_spec.rb | 2 +- spec/acceptance/realtime/connection_failures_spec.rb | 12 ++++++------ spec/acceptance/rest/base_spec.rb | 9 ++++----- spec/acceptance/rest/client_spec.rb | 6 +++--- 4 files changed, 14 insertions(+), 15 deletions(-) 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..84a3967f 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 @@ -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 From 9db3a618ed8277444c05d3b3fcf70d42bcd8ec30 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 11 Jun 2025 12:16:05 -0300 Subject: [PATCH 5/6] Change expectation of status code for invalid key name I guess they must have changed it on the backend to make the statusCode match the code? --- spec/acceptance/realtime/connection_failures_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/acceptance/realtime/connection_failures_spec.rb b/spec/acceptance/realtime/connection_failures_spec.rb index 84a3967f..64a1225b 100644 --- a/spec/acceptance/realtime/connection_failures_spec.rb +++ b/spec/acceptance/realtime/connection_failures_spec.rb @@ -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 From dbd47b8dc86773c27344067a1f97309f76a3679e Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 11 Jun 2025 15:31:50 -0300 Subject: [PATCH 6/6] Don't use fallback hosts for Realtime token errors This is a partial fix for #444 (nature of error not considered when deciding whether to use a fallback host for realtime connections). Here, I implement just enough of RTN17f to make sure that we don't use a fallback host in response to a token error. This fixes an intermittent failure in the test case on connection_failures_spec.rb:1361, which tests that we correctly handle token errors when resuming a connection per RTN15c5. This test was intermittently failing because it would incorrectly try and use a fallback host after receiving the token error, causing the resume attempt to sometimes fail with "Unable to resume connection from another site". I don't have loads of time on to spend on this issue right now, which is why I'm applying the smallest possible fix. We need to return to #444 and properly consider how to identify the types of errors mentioned in RTN17f. --- lib/ably/realtime/connection.rb | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) 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