Commit 4c22218a authored by Dmytro Zaporozhets's avatar Dmytro Zaporozhets

Merge branch 'sentry-1350443' into 'master'

Record JSON parser errors due to invalid reports

See merge request gitlab-org/gitlab!26944
parents 68c8622b 195e3aad
---
title: Stop recording JSON parser errors due to third party license scanning integrations
merge_request: 26944
author:
type: fixed
...@@ -17,11 +17,17 @@ module Gitlab ...@@ -17,11 +17,17 @@ module Gitlab
parser = PARSERS.fetch(report.major_version) parser = PARSERS.fetch(report.major_version)
parser.new(report).parse(json) parser.new(report).parse(json)
rescue JSON::ParserError rescue JSON::ParserError => error
raise LicenseScanningParserError, 'JSON parsing failed' Gitlab::ErrorTracking.track_exception(error, error_details_for(json_data))
rescue => e end
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
raise LicenseScanningParserError, 'License scanning report parsing failed' private
def error_details_for(json)
return { message: 'artifact is blank' } if json.blank?
return { message: "artifact is too big (#{json.bytesize} bytes)" } if json.bytesize > 1.megabyte
{ message: 'artifact is not JSON', raw: json }
end end
end end
end end
......
...@@ -173,11 +173,12 @@ RSpec.describe Gitlab::Ci::Parsers::LicenseCompliance::LicenseScanning do ...@@ -173,11 +173,12 @@ RSpec.describe Gitlab::Ci::Parsers::LicenseCompliance::LicenseScanning do
end end
context 'when the report is not a valid JSON document' do context 'when the report is not a valid JSON document' do
it do before do
expect do subject.parse!('invalid', report)
subject.parse!('blah', report)
end.to raise_error(Gitlab::Ci::Parsers::LicenseCompliance::LicenseScanning::LicenseScanningParserError)
end end
it { expect(report.version).to eql('1.0') }
it { expect(report.licenses).to be_empty }
end end
end end
end end
...@@ -252,8 +252,9 @@ RSpec.describe Ci::Build do ...@@ -252,8 +252,9 @@ RSpec.describe Ci::Build do
create(:ee_ci_job_artifact, :license_scan, :with_corrupted_data, job: job, project: job.project) create(:ee_ci_job_artifact, :license_scan, :with_corrupted_data, job: job, project: job.project)
end end
it 'raises an error' do it 'returns an empty report' do
expect { subject }.to raise_error(Gitlab::Ci::Parsers::LicenseCompliance::LicenseScanning::LicenseScanningParserError) expect { subject }.not_to raise_error
expect(license_scanning_report).to be_empty
end end
end end
......
...@@ -52,16 +52,16 @@ describe Ci::CompareLicenseScanningReportsService do ...@@ -52,16 +52,16 @@ describe Ci::CompareLicenseScanningReportsService do
let!(:base_pipeline) { build(:ee_ci_pipeline, :with_corrupted_license_scanning_report, project: project) } let!(:base_pipeline) { build(:ee_ci_pipeline, :with_corrupted_license_scanning_report, project: project) }
let!(:head_pipeline) { build(:ee_ci_pipeline, :with_corrupted_license_scanning_report, project: project) } let!(:head_pipeline) { build(:ee_ci_pipeline, :with_corrupted_license_scanning_report, project: project) }
it 'returns status and error message' do it 'does not expose parser errors' do
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:parsed)
expect(subject[:status_reason]).to include('JSON parsing failed')
end end
it 'returns status and error message when pipeline is nil' do context "when the base pipeline is nil" do
result = service.execute(nil, head_pipeline) subject { service.execute(nil, head_pipeline) }
expect(result[:status]).to eq(:error) it 'does not expose parser errors' do
expect(result[:status_reason]).to include('JSON parsing failed') expect(subject[:status]).to eq(:parsed)
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment