error handling for settings api

lib/datadog/ci/transport/api_client.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "json"
+
 require "datadog/core/environment/identity"
 
 require_relative "../ext/transport"
@@ -12,19 +14,52 @@ module Transport
       #
       # TODO: Rename ApiClient to SettingsApiClient
       class ApiClient
+        class Response
+          def initialize(http_response)
+            @http_response = http_response
+            @json = nil
+          end
+
+          def ok?
+            resp = @http_response
+            !resp.nil? && resp.ok?
+          end
+
+          def payload
+            return @json if @json
+
+            resp = @http_response
+            return default_payload if resp.nil? || !resp.ok?
+
+            begin
+              @json = JSON.parse(resp.payload).dig("data", "attributes")
+            rescue JSON::ParserError => e
+              Datadog.logger.error("Failed to parse settings response payload: #{e}. Payload was: #{resp.payload}")
+              @json = default_payload
+            end
+          end
+
+          private
+
+          def default_payload
+            {"itr_enabled" => false}
+          end
+        end
+
         def initialize(api: nil, dd_env: nil)
           @api = api
           @dd_env = dd_env
         end
 
         def fetch_library_settings(test_session)
-          # TODO: return error response if api is not present
           api = @api
-          return {} unless api
-          # TODO: return error response - use some wrapper from ddtrace as an example
-          api.api_request(
-            path: Ext::Transport::DD_API_SETTINGS_PATH,
-            payload: payload(test_session)
+          return Response.new(nil) unless api
+
+          Response.new(
+            api.api_request(
+              path: Ext::Transport::DD_API_SETTINGS_PATH,
+              payload: payload(test_session)
+            )
           )
         end
 

sig/datadog/ci/transport/api_client.rbs

@@ -2,12 +2,25 @@ module Datadog
   module CI
     module Transport
       class ApiClient
+        class Response
+          @http_response: Datadog::Core::Transport::HTTP::Adapters::Net::Response?
+          @json: Hash[String, untyped]?
+
+          def initialize: (Datadog::Core::Transport::HTTP::Adapters::Net::Response? http_response) -> void
+
+          def ok?: () -> bool
+
+          private
+
+          def default_payload: () -> Hash[String, untyped]
+        end
+
         @api: Datadog::CI::Transport::Api::Base?
         @dd_env: String?
 
         def initialize: (?api: Datadog::CI::Transport::Api::Base?, ?dd_env: String?) -> void
 
-        def fetch_library_settings: (Datadog::CI::TestSession test_session) -> untyped
+        def fetch_library_settings: (Datadog::CI::TestSession test_session) -> Response
 
         private
 

spec/datadog/ci/transport/api_client_spec.rb

@@ -4,7 +4,7 @@
   let(:api) { spy("api") }
   let(:dd_env) { "ci" }
 
-  subject { described_class.new(api: api, dd_env: dd_env) }
+  subject(:client) { described_class.new(api: api, dd_env: dd_env) }
 
   describe "#fetch_library_settings" do
     let(:service) { "service" }
@@ -26,7 +26,7 @@
     let(:path) { Datadog::CI::Ext::Transport::DD_API_SETTINGS_PATH }
 
     it "requests the settings" do
-      subject.fetch_library_settings(test_session)
+      client.fetch_library_settings(test_session)
 
       expect(api).to have_received(:api_request) do |args|
         expect(args[:path]).to eq(path)
@@ -50,5 +50,89 @@
         expect(configurations["runtime.version"]).to eq("runtime_version")
       end
     end
+
+    context "parsing response" do
+      subject(:response) { client.fetch_library_settings(test_session) }
+
+      context "when api is present" do
+        before do
+          allow(api).to receive(:api_request).and_return(http_response)
+        end
+
+        context "when response is OK" do
+          let(:http_response) do
+            double(
+              "http_response",
+              ok?: true,
+              payload: {
+                "data" => {
+                  "id" => "123",
+                  "type" => Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE,
+                  "attributes" => {
+                    "code_coverage" => true,
+                    "tests_skipping" => false,
+                    "itr_enabled" => true,
+                    "require_git" => false
+                  }
+                }
+              }.to_json
+            )
+          end
+
+          it "parses the response" do
+            expect(response.ok?).to be true
+            expect(response.payload).to eq({
+              "code_coverage" => true,
+              "tests_skipping" => false,
+              "itr_enabled" => true,
+              "require_git" => false
+            })
+          end
+        end
+
+        context "when response is not OK" do
+          let(:http_response) do
+            double(
+              "http_response",
+              ok?: false,
+              payload: ""
+            )
+          end
+
+          it "parses the response" do
+            expect(response.ok?).to be false
+            expect(response.payload).to eq("itr_enabled" => false)
+          end
+        end
+
+        context "when response is OK but JSON is malformed" do
+          let(:http_response) do
+            double(
+              "http_response",
+              ok?: true,
+              payload: "not json"
+            )
+          end
+
+          before do
+            expect(Datadog.logger).to receive(:error).with(/Failed to parse settings response payload/)
+          end
+
+          it "parses the response" do
+            expect(response.ok?).to be true
+            expect(response.payload).to eq("itr_enabled" => false)
+          end
+        end
+      end
+
+      context "when there is no api" do
+        let(:api) { nil }
+
+        it "returns an empty response" do
+          expect(response.ok?).to be false
+          expect(response.payload).to eq("itr_enabled" => false)
+        end
+      end
+    end
   end
 end