Merge pull request #2821 from newrelic/james_best_face_barney_kessel_…

…hands

ServerlessHandler: error handling for URI parsing

.rubocop.yml

@@ -1250,6 +1250,7 @@ Style/MethodCallWithArgsParentheses:
     - add_development_dependency
     - catch
     - debug
+    - error
     - exit
     - expect
     - fail

lib/new_relic/agent/serverless_handler.rb

@@ -186,13 +186,18 @@ def process_api_gateway_info
       def http_uri(info)
         return unless info[:host] && info[:path]
 
-        url_str = "https://#{info[:host]}:#{info[:port]}#{info[:path]}"
+        url_str = "https://#{info[:host]}"
+        url_str += ":#{info[:port]}" unless info[:host].match?(':')
+        url_str += "#{info[:path]}"
+
         if info[:query_parameters]
           qp = info[:query_parameters].map { |k, v| "#{k}=#{v}" }.join('&')
           url_str += "?#{qp}"
         end
 
         URI.parse(url_str)
+      rescue StandardError => e
+        NewRelic::Agent.logger.error "ServerlessHandler failed to parse the source HTTP URI: #{e}"
       end
 
       def info_for_api_gateway_v2

test/new_relic/agent/serverless_handler_test.rb

@@ -423,6 +423,47 @@ def test_metadata_for_payload_v2
         refute_empty metadata[:agent_language]
       end
 
+      # when testing locally without AWS the base host value may contain
+      # ':3000' so make sure the constructed URI doesn't end up as
+      # 'https://localhost:3000:443'
+      def test_http_uri_with_existing_port
+        info = {:host => 'localhost:3000',
+                :port => 443,
+                :path => ''}
+
+        uri = fresh_handler.send(:http_uri, info)
+
+        assert_equal 'https://localhost:3000', uri.to_s
+      end
+
+      def test_http_uri_still_adds_the_port_when_needed
+        info = {:host => 'flame',
+                :port => '1138',
+                :path => '/broiler'}
+
+        uri = fresh_handler.send(:http_uri, info)
+
+        assert_equal 'https://flame:1138/broiler', uri.to_s
+      end
+
+      def test_http_uri_handles_errors
+        info = {:host => 'perfecto',
+                :port => 443,
+                :path => ''}
+        logger_mock = Minitest::Mock.new
+        logger_mock.expect :error, nil, [/failed to parse/]
+
+        URI.stub :parse, proc { |_uri| raise 'kaboom' } do
+          NewRelic::Agent.stub :logger, logger_mock do
+            uri = fresh_handler.send(:http_uri, info)
+
+            assert_nil uri, 'Expected http_uri to rescue and return nil'
+          end
+        end
+
+        logger_mock.verify
+      end
+
       private
 
       def handler