Merge pull request #133 from ably/add-request-id-to-path

Add request_id option to client

lib/ably/exceptions.rb

@@ -13,7 +13,7 @@ module Exceptions
     # @!attribute [r] code
     #   @return [String] Ably specific error code
     class BaseAblyException < StandardError
-      attr_reader :status, :code
+      attr_reader :status, :code, :request_id
 
       def initialize(message, status = nil, code = nil, base_exception = nil, options = {})
         super message
@@ -25,6 +25,7 @@ def initialize(message, status = nil, code = nil, base_exception = nil, options
         @code = code
         @code ||= base_exception.code if base_exception && base_exception.respond_to?(:code)
         @code ||= options[:fallback_code]
+        @request_id ||= options[:request_id]
       end
 
       def to_s
@@ -34,6 +35,7 @@ def to_s
           additional_info << "code: #{code}" if code
           additional_info << "http status: #{status}" if status
           additional_info << "base exception: #{@base_exception.class}" if @base_exception
+          additional_info << "request_id: #{request_id}" if request_id
           message << "(#{additional_info.join(', ')})"
         end
         message.join(' ')

lib/ably/rest/client.rb

@@ -83,6 +83,10 @@ class Client
       # if empty or nil then fallback host functionality is disabled
       attr_reader :fallback_hosts
 
+      # Whethere the {Client} has to add a random identifier to the path of a request
+      # @return [Boolean]
+      attr_reader :add_request_ids
+
       # Creates a {Ably::Rest::Client Rest Client} and configures the {Ably::Auth} object for the connection.
       #
       # @param [Hash,String] options an options Hash used to configure the client and the authentication, or String with an API key or Token ID
@@ -146,6 +150,7 @@ def initialize(options)
         @custom_host      = options.delete(:rest_host)
         @custom_port      = options.delete(:port)
         @custom_tls_port  = options.delete(:tls_port)
+        @add_request_ids  = options.delete(:add_request_ids)
 
         if options[:fallback_hosts_use_default] && options[:fallback_jhosts]
           raise ArgumentError, "fallback_hosts_use_default cannot be set to trye when fallback_jhosts is also provided"
@@ -434,11 +439,25 @@ def send_request(method, path, params, options)
         max_retry_duration = http_defaults.fetch(:max_retry_duration)
         requested_at       = Time.now
         retry_count        = 0
+        request_id         = nil
+        if add_request_ids
+          params = if params.nil?
+            {}
+          else
+            params.dup
+          end
+          request_id = SecureRandom.urlsafe_base64(10)
+          params[:request_id] = request_id
+        end
 
         begin
           use_fallback = can_fallback_to_alternate_ably_host? && retry_count > 0
 
           connection(use_fallback: use_fallback).send(method, path, params) do |request|
+            if add_request_ids
+              request.options.context = {} if request.options.context.nil?
+              request.options.context[:request_id] = request_id
+            end
             unless options[:send_auth_header] == false
               request.headers[:authorization] = auth.auth_header
               if options[:headers]
@@ -456,12 +475,12 @@ def send_request(method, path, params, options)
             logger.warn { "Ably::Rest::Client - Retry #{retry_count} for #{method} #{path} #{params} as initial attempt failed: #{error}" }
             retry
           end
-
           case error
             when Faraday::TimeoutError
-              raise Ably::Exceptions::ConnectionTimeout.new(error.message, nil, 80014, error)
+              raise Ably::Exceptions::ConnectionTimeout.new(error.message, nil, 80014, error, { request_id: request_id })
             when Faraday::ClientError
-              raise Ably::Exceptions::ConnectionError.new(error.message, nil, 80000, error)
+              # request_id is also available in the request context
+              raise Ably::Exceptions::ConnectionError.new(error.message, nil, 80000, error, { request_id: request_id })
             else
               raise error
           end

lib/ably/rest/middleware/exceptions.rb

@@ -27,8 +27,8 @@ def on_complete(env)
             end
 
             message = 'Unknown server error' if message.to_s.strip == ''
-
-            exception_args = [message, error_status_code, error_code]
+            request_id = env.request.context[:request_id] if env.request.context
+            exception_args = [message, error_status_code, error_code, nil, { request_id: request_id }]
 
             if env.status >= 500
               raise Ably::Exceptions::ServerError.new(*exception_args)

spec/acceptance/rest/client_spec.rb

@@ -959,5 +959,100 @@ def encode64(text)
         end
       end
     end
+
+    context 'request_id generation' do
+      context 'Timeout error' do
+        context 'with request_id', :webmock do
+          let(:custom_logger) do
+            Class.new do
+              def initialize
+                @messages = []
+              end
+
+              [:fatal, :error, :warn, :info, :debug].each do |severity|
+                define_method severity do |message, &block|
+                  message_val = [message]
+                  message_val << block.call if block
+
+                  @messages << [severity, message_val.compact.join(' ')]
+                end
+              end
+
+              def logs
+                @messages
+              end
+
+              def level
+                1
+              end
+
+              def level=(new_level)
+              end
+            end
+          end
+          let(:custom_logger_object) { custom_logger.new }
+          let(:client_options) { default_options.merge(key: api_key, logger: custom_logger_object, add_request_ids: true) }
+          before do
+            @request_id = nil
+            stub_request(:get, Addressable::Template.new("#{client.endpoint}/time{?request_id}")).with do |request|
+              @request_id = request.uri.query_values['request_id']
+            end.to_return do
+              raise Faraday::TimeoutError.new('timeout error message')
+            end
+          end
+          it 'has an error with the same request_id of the request' do
+            expect{ client.time }.to raise_error(Ably::Exceptions::ConnectionTimeout, /#{@request_id}/)
+            expect(custom_logger_object.logs.find { |severity, message| message.match(/#{@request_id}/i)} ).to_not be_nil
+          end
+        end
+
+        context 'when specifying fallback hosts', :webmock do
+          let(:client_options) { { key: api_key, fallback_hosts_use_default: true, add_request_ids: true } }
+          let(:requests)       { [] }
+          before do
+            @request_id = nil
+            hosts = Ably::FALLBACK_HOSTS + ['rest.ably.io']
+            hosts.each do |host|
+              stub_request(:get, Addressable::Template.new("https://#{host.downcase}/time{?request_id}")).with do |request|
+                @request_id = request.uri.query_values['request_id']
+                requests << @request_id
+              end.to_return do
+                raise Faraday::TimeoutError.new('timeout error message')
+              end
+            end
+          end
+          it 'request_id is the same across retries' do
+            expect{ client.time }.to raise_error(Ably::Exceptions::ConnectionTimeout, /#{@request_id}/)
+            expect(requests.uniq.count).to eql(1)
+            expect(requests.uniq.first).to eql(@request_id)
+          end
+        end
+
+        context 'without request_id' do
+          let(:client_options) { default_options.merge(key: api_key, http_request_timeout: 0) }
+          it 'does not include request_id in ConnectionTimeout error' do
+            begin
+              client.stats
+            rescue Ably::Exceptions::ConnectionTimeout => err
+              expect(err.request_id).to eql(nil)
+            end
+          end
+        end
+      end 
+
+      context 'UnauthorizedRequest nonce error' do
+        let(:token_params) { { nonce: "samenonce_#{protocol}", timestamp:  Time.now.to_i } }
+        it 'includes request_id in UnauthorizedRequest error due to replayed nonce' do
+          client1 = Ably::Rest::Client.new(default_options.merge(key: api_key))
+          client2 = Ably::Rest::Client.new(default_options.merge(key: api_key, add_request_ids: true))
+          expect { client1.auth.request_token(token_params) }.not_to raise_error
+          begin
+            client2.auth.request_token(token_params)
+          rescue Ably::Exceptions::UnauthorizedRequest => err
+            expect(err.request_id).to_not eql(nil)
+          end
+        end
+      end
+    end
   end
 end

spec/unit/rest/client_spec.rb

@@ -50,4 +50,11 @@
       end
     end
   end
+
+  context 'request_id generation' do
+    let(:client_options) { { key: 'appid.keyuid:keysecret', add_request_ids: true } }
+    it 'includes request_id in URL' do
+      expect(subject.add_request_ids).to eql(true)
+    end
+  end
 end