Add handlers for non 200 status codes

This deals with the fact that the JiraConnect response format depends on
the response code.

app/services/jira_connect/sync_service.rb

@@ -31,7 +31,7 @@ module JiraConnect
         jira_response: response&.to_json
       }
 
-      if response && response['errorMessages']
+      if response && response['errorMessages'].present?
         logger.error(message)
       else
         logger.info(message)

lib/atlassian/jira_connect/client.rb

@@ -45,11 +45,14 @@ module Atlassian
           properties: { projectId: "project-#{project.id}" }
         })
 
-        if r['failedFeatureFlags'].present?
-          errors = r['failedFeatureFlags'].flat_map do |k, errs|
-            errs.map { |e| "#{k}: #{e['message']}" }
+        handle_response(r, 'feature flags') do |data|
+          failed = data['failedFeatureFlags']
+          if failed.present?
+            errors = failed.flat_map do |k, errs|
+              errs.map { |e| "#{k}: #{e['message']}" }
+            end
+            { 'errorMessages' => errors }
           end
-          { 'errorMessages' => errors }
         end
       end
 
@@ -62,7 +65,7 @@ module Atlassian
         return if items.empty?
 
         r = post('/rest/deployments/0.1/bulk', { deployments: items })
-        errors(r, 'rejectedDeployments')
+        handle_response(r, 'deployments') { |data| errors(data, 'rejectedDeployments') }
       end
 
       def store_build_info(project:, pipelines:, update_sequence_id: nil)
@@ -80,7 +83,7 @@ module Atlassian
         return if builds.empty?
 
         r = post('/rest/builds/0.1/bulk', { builds: builds })
-        errors(r, 'rejectedBuilds')
+        handle_response(r, 'builds') { |data| errors(data, 'rejectedBuilds') }
       end
 
       def store_dev_info(project:, commits: nil, branches: nil, merge_requests: nil, update_sequence_id: nil)
@@ -113,8 +116,23 @@ module Atlassian
         { providerMetadata: { product: "GitLab #{Gitlab::VERSION}" } }
       end
 
-      def errors(response, key)
+      def handle_response(response, name, &block)
         data = response.parsed_response
+
+        case response.code
+        when 200 then yield data
+        when 400 then { 'errorMessages' => data.map { |e| e['message'] } }
+        when 401 then { 'errorMessages' => ['Invalid JWT'] }
+        when 403 then { 'errorMessages' => ["App does not support #{name}"] }
+        when 413 then { 'errorMessages' => ['Data too large'] + data.map { |e| e['message'] } }
+        when 429 then { 'errorMessages' => ['Rate limit exceeded'] }
+        when 503 then { 'errorMessages' => ['Service unavailable'] }
+        else
+          { 'errorMessages' => ['Unknown error'], 'response' => data }
+        end
+      end
+
+      def errors(data, key)
         messages = if data[key].present?
                      data[key].flat_map do |rejection|
                        rejection['errors'].map { |e| e['message'] }

spec/lib/atlassian/jira_connect/client_spec.rb

@@ -107,6 +107,75 @@ RSpec.describe Atlassian::JiraConnect::Client do
     }
   end
 
+  describe '#handle_response' do
+    let(:errors) { [{ 'message' => 'X' }, { 'message' => 'Y' }] }
+    let(:processed) { subject.send(:handle_response, response, 'foo') { |x| [:data, x] } }
+
+    context 'the response is 200 OK' do
+      let(:response) { double(code: 200, parsed_response: :foo) }
+
+      it 'yields to the block' do
+        expect(processed).to eq [:data, :foo]
+      end
+    end
+
+    context 'the response is 400 bad request' do
+      let(:response) { double(code: 400, parsed_response: errors) }
+
+      it 'extracts the errors messages' do
+        expect(processed).to eq('errorMessages' => %w(X Y))
+      end
+    end
+
+    context 'the response is 401 forbidden' do
+      let(:response) { double(code: 401, parsed_response: nil) }
+
+      it 'reports that our JWT is wrong' do
+        expect(processed).to eq('errorMessages' => ['Invalid JWT'])
+      end
+    end
+
+    context 'the response is 403' do
+      let(:response) { double(code: 403, parsed_response: nil) }
+
+      it 'reports that the App is misconfigured' do
+        expect(processed).to eq('errorMessages' => ['App does not support foo'])
+      end
+    end
+
+    context 'the response is 413' do
+      let(:response) { double(code: 413, parsed_response: errors) }
+
+      it 'extracts the errors messages' do
+        expect(processed).to eq('errorMessages' => ['Data too large', 'X', 'Y'])
+      end
+    end
+
+    context 'the response is 429' do
+      let(:response) { double(code: 429, parsed_response: nil) }
+
+      it 'reports that we exceeded the rate limit' do
+        expect(processed).to eq('errorMessages' => ['Rate limit exceeded'])
+      end
+    end
+
+    context 'the response is 503' do
+      let(:response) { double(code: 503, parsed_response: nil) }
+
+      it 'reports that the service is unavailable' do
+        expect(processed).to eq('errorMessages' => ['Service unavailable'])
+      end
+    end
+
+    context 'the response is anything else' do
+      let(:response) { double(code: 1000, parsed_response: :something) }
+
+      it 'reports that this was unanticipated' do
+        expect(processed).to eq('errorMessages' => ['Unknown error'], 'response' => :something)
+      end
+    end
+  end
+
   describe '#store_deploy_info' do
     let_it_be(:environment) { create(:environment, name: 'DEV', project: project) }
     let_it_be(:deployments) do

spec/services/jira_connect/sync_service_spec.rb

@@ -45,11 +45,11 @@ RSpec.describe JiraConnect::SyncService do
       it 'logs the response as an error' do
         expect_next(client).to store_info([
           { 'errorMessages' => ['some error message'] },
-          { 'rejectedBuilds' => ['x'] }
+          { 'errorMessages' => ['x'] }
         ])
 
         expect_log(:error, { 'errorMessages' => ['some error message'] })
-        expect_log(:error, { 'rejectedBuilds' => ['x'] })
+        expect_log(:error, { 'errorMessages' => ['x'] })
 
         subject
       end