Merge pull request #443 from ably/2025-06-10-fix-ci

Fix decoding of MessagePack data, and other fixes to CI

Author: lawrence-forooghian

.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

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'
 

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

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

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

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
 

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

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