[spaceship] parse response body as json if not already (#18766)

* parse response body as json if not already

parse response body as json if not already

* parse if response.body is type of String

* fixes broken test

* check if response is empty and use empty json object if so, else try parse the body

* Clean up logic and removed unneeded if statements and added tests

* Add test for string json

Co-authored-by: Muhammet Ilendemli <mi@tailored-apps.com>
Co-authored-by: Josh Holtz <josh@rokkincat.com>

spaceship/lib/spaceship/connect_api/api_client.rb

@@ -207,16 +207,19 @@ def handle_response(response)
 
       # Overridden from Spaceship::Client
       def handle_error(response)
+        body = response.body.empty? ? {} : response.body
+        body = JSON.parse(body) if body.kind_of?(String)
+
         case response.status.to_i
         when 401
-          raise UnauthorizedAccessError, format_errors(response) if response && (response.body || {})['errors']
+          raise UnauthorizedAccessError, format_errors(response)
         when 403
-          error = (response.body['errors'] || []).first || {}
+          error = (body['errors'] || []).first || {}
           error_code = error['code']
           if error_code == "FORBIDDEN.REQUIRED_AGREEMENTS_MISSING_OR_EXPIRED"
-            raise ProgramLicenseAgreementUpdated, format_errors(response) if response && (response.body || {})['errors']
+            raise ProgramLicenseAgreementUpdated, format_errors(response)
           else
-            raise AccessForbiddenError, format_errors(response) if response && (response.body || {})['errors']
+            raise AccessForbiddenError, format_errors(response)
           end
         end
       end
@@ -280,7 +283,23 @@ def format_errors(response)
         #   ]
         # }
 
-        return response.body['errors'].map do |error|
+        # Membership expired
+        # {
+        #   "errors" : [
+        #     {
+        #       "id" : "UUID",
+        #       "status" : "403",
+        #       "code" : "FORBIDDEN_ERROR",
+        #       "title" : "This request is forbidden for security reasons",
+        #       "detail" : "Team ID: 'ID' is not associated with an active membership. To check your teams membership status, sign in your account on the developer website. https://developer.apple.com/account/"
+        #     }
+        #   ]
+        # }
+
+        body = response.body.empty? ? {} : response.body
+        body = JSON.parse(body) if body.kind_of?(String)
+
+        formatted_errors = (body['errors'] || []).map do |error|
           messages = [[error['title'], error['detail'], error.dig("source", "pointer")].compact.join(" - ")]
 
           meta = error["meta"] || {}
@@ -290,6 +309,12 @@ def format_errors(response)
             [[associated_error["title"], associated_error["detail"]].compact.join(" - ")]
           end
         end.flatten.join("\n")
+
+        if formatted_errors.empty?
+          formatted_errors << "Unknown error"
+        end
+
+        return formatted_errors
       end
 
       private

spaceship/spec/connect_api/api_client_spec.rb

@@ -125,4 +125,97 @@ def stub_client_request(uri, status, body)
       end.to raise_error(Spaceship::AccessForbiddenError)
     end
   end
+
+  describe "#handle_error" do
+    let(:mock_token) { double('token') }
+    let(:client) { Spaceship::ConnectAPI::APIClient.new(token: mock_token) }
+    let(:mock_response) { double('response') }
+
+    describe "status of 200" do
+      before(:each) do
+        allow(mock_response).to receive(:status).and_return(200)
+      end
+
+      it 'does not raise' do
+        allow(mock_response).to receive(:body).and_return({})
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to_not(raise_error)
+      end
+    end
+
+    describe "status of 401" do
+      before(:each) do
+        allow(mock_response).to receive(:status).and_return(401)
+      end
+
+      it 'raises UnauthorizedAccessError with no errors in body' do
+        allow(mock_response).to receive(:body).and_return({})
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to raise_error(Spaceship::UnauthorizedAccessError, /Unknown error/)
+      end
+
+      it 'raises UnauthorizedAccessError when body is string' do
+        allow(mock_response).to receive(:body).and_return('{"errors":[{"title": "Some title", "detail": "some detail"}]}')
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to raise_error(Spaceship::UnauthorizedAccessError, /Some title - some detail/)
+      end
+    end
+
+    describe "status of 403" do
+      before(:each) do
+        allow(mock_response).to receive(:status).and_return(403)
+      end
+
+      it 'raises ProgramLicenseAgreementUpdated with no errors in body FORBIDDEN.REQUIRED_AGREEMENTS_MISSING_OR_EXPIRED' do
+        allow(mock_response).to receive(:body).and_return({
+          "errors" => [
+            {
+              "code" => "FORBIDDEN.REQUIRED_AGREEMENTS_MISSING_OR_EXPIRED"
+            }
+          ]
+        })
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to raise_error(Spaceship::ProgramLicenseAgreementUpdated)
+      end
+
+      it 'raises AccessForbiddenError with no errors in body' do
+        allow(mock_response).to receive(:body).and_return({})
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to raise_error(Spaceship::AccessForbiddenError, /Unknown error/)
+      end
+
+      it 'raises AccessForbiddenError when body is string' do
+        allow(mock_response).to receive(:body).and_return('{"errors":[{"title": "Some title", "detail": "some detail"}]}')
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to raise_error(Spaceship::AccessForbiddenError, /Some title - some detail/)
+      end
+
+      it 'raises AccessForbiddenError with errors in body' do
+        allow(mock_response).to receive(:body).and_return({
+          "errors" => [
+            {
+              "title" => "Some title",
+              "detail" => "some detail"
+            }
+          ]
+        })
+
+        expect do
+          client.send(:handle_error, mock_response)
+        end.to raise_error(Spaceship::AccessForbiddenError, /Some title - some detail/)
+      end
+    end
+  end
 end