Merge branch 'better-error-handling-for-report-comparison' into 'master'

Refactor error handling for report comparison

See merge request gitlab-org/gitlab!17516

ee/app/finders/security/pipeline_vulnerabilities_finder.rb

@@ -24,24 +24,22 @@ module Security
     end
 
     def execute
-      pipeline_reports.each_with_object([]) do |(type, report), occurrences|
+      pipeline_reports&.each_with_object([]) do |(type, report), occurrences|
         next unless requested_type?(type)
 
+        raise ParseError, 'JSON parsing failed' if report.error.is_a?(Gitlab::Ci::Parsers::Security::Common::SecurityReportParserError)
+
         normalized_occurrences = normalize_report_occurrences(report.occurrences)
         filtered_occurrences = filter(normalized_occurrences)
 
         occurrences.concat(filtered_occurrences)
       end
-
-    # Created follow-up issue to better handle exception case - https://gitlab.com/gitlab-org/gitlab/issues/14007
-    rescue NoMethodError => _ # propagate error for CompareReportsBaseService
-      raise ParseError, 'JSON parsing failed'
     end
 
     private
 
     def pipeline_reports
-      pipeline.security_reports.reports
+      pipeline&.security_reports&.reports
     end
 
     def normalize_report_occurrences(report_occurrences)

ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb

@@ -59,13 +59,20 @@ describe Ci::CompareContainerScanningReportsService do
     end
 
     context 'when head pipeline has corrupted container scanning vulnerability reports' do
-      let!(:base_pipeline) { nil }
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) }
       let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) }
 
       it 'returns status and error message' do
         expect(subject[:status]).to eq(:error)
         expect(subject[:status_reason]).to include('JSON parsing failed')
       end
+
+      it 'returns status and error message when pipeline is nil' do
+        result = service.execute(nil, head_pipeline)
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:status_reason]).to include('JSON parsing failed')
+      end
     end
   end
 end

ee/spec/services/ci/compare_dependency_scanning_reports_service_spec.rb

@@ -61,13 +61,20 @@ describe Ci::CompareDependencyScanningReportsService do
     end
 
     context 'when head pipeline has corrupted dependency scanning vulnerability reports' do
-      let!(:base_pipeline) { nil }
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project) }
       let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project) }
 
       it 'returns status and error message' do
         expect(subject[:status]).to eq(:error)
         expect(subject[:status_reason]).to include('JSON parsing failed')
       end
+
+      it 'returns status and error message when pipeline is nil' do
+        result = service.execute(nil, head_pipeline)
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:status_reason]).to include('JSON parsing failed')
+      end
     end
   end
 end

ee/spec/services/ci/compare_license_management_reports_service_spec.rb

@@ -50,13 +50,20 @@ describe Ci::CompareLicenseManagementReportsService do
     end
 
     context 'when head pipeline has corrupted license management reports' do
-      let!(:base_pipeline) { nil }
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_corrupted_license_management_report, project: project) }
       let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_license_management_report, project: project) }
 
       it 'returns status and error message' do
         expect(subject[:status]).to eq(:error)
         expect(subject[:status_reason]).to include('JSON parsing failed')
       end
+
+      it 'returns status and error message when pipeline is nil' do
+        result = service.execute(nil, head_pipeline)
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:status_reason]).to include('JSON parsing failed')
+      end
     end
   end
 end