From ad6ac17c5434f7eb87005dc3603b4ae9409c333f Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Fri, 28 Apr 2017 15:20:58 -0500 Subject: [PATCH 1/4] Added rescue block for the test method for the prometheus service --- .../unreleased/prometheus-integration-test-setting-fix.yml | 4 ++++ lib/gitlab/prometheus.rb | 6 +++++- spec/models/project_services/prometheus_service_spec.rb | 5 +++-- 3 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/prometheus-integration-test-setting-fix.yml diff --git a/changelogs/unreleased/prometheus-integration-test-setting-fix.yml b/changelogs/unreleased/prometheus-integration-test-setting-fix.yml new file mode 100644 index 00000000000..c65c682a114 --- /dev/null +++ b/changelogs/unreleased/prometheus-integration-test-setting-fix.yml @@ -0,0 +1,4 @@ +--- +title: Added rescue block for the test method +merge_request: 10994 +author: diff --git a/lib/gitlab/prometheus.rb b/lib/gitlab/prometheus.rb index 62239779454..37afec5e7df 100644 --- a/lib/gitlab/prometheus.rb +++ b/lib/gitlab/prometheus.rb @@ -49,7 +49,11 @@ module Gitlab end def get(url) - handle_response(HTTParty.get(url)) + begin + handle_response(HTTParty.get(url)) + rescue SocketError + raise PrometheusError, "Can't connect to #{url}" + end end def handle_response(response) diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index d15079b686b..5ef1b53be15 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -93,11 +93,12 @@ describe PrometheusService, models: true, caching: true do [404, 500].each do |status| context "when Prometheus responds with #{status}" do + body_response = 'QUERY_FAILED' before do - stub_all_prometheus_requests(environment.slug, status: status, body: 'QUERY FAILED!') + stub_all_prometheus_requests(environment.slug, status: status, body: body_response) end - it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } + it { is_expected.to eq(success: false, result: %(#{status} - \"#{body_response}\")) } end end end From 63a5d98a7c921289e9b43f4b54c03614427f7eda Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Tue, 2 May 2017 11:16:59 -0500 Subject: [PATCH 2/4] Added specs --- lib/gitlab/prometheus.rb | 10 +++++----- spec/lib/gitlab/prometheus_spec.rb | 18 ++++++++++++++++++ spec/support/prometheus_helpers.rb | 10 ++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/prometheus.rb b/lib/gitlab/prometheus.rb index 37afec5e7df..94541009100 100644 --- a/lib/gitlab/prometheus.rb +++ b/lib/gitlab/prometheus.rb @@ -49,11 +49,11 @@ module Gitlab end def get(url) - begin - handle_response(HTTParty.get(url)) - rescue SocketError - raise PrometheusError, "Can't connect to #{url}" - end + handle_response(HTTParty.get(url)) + rescue SocketError + raise PrometheusError, "Can't connect to #{url}" + rescue OpenSSL::SSL::SSLError + raise PrometheusError, "#{url} contains invalid SSL data" end def handle_response(response) diff --git a/spec/lib/gitlab/prometheus_spec.rb b/spec/lib/gitlab/prometheus_spec.rb index 280264188e2..d8683669518 100644 --- a/spec/lib/gitlab/prometheus_spec.rb +++ b/spec/lib/gitlab/prometheus_spec.rb @@ -49,6 +49,24 @@ describe Gitlab::Prometheus, lib: true do end end + describe 'failure to reach a prometheus url' do + prometheus_invalid_url = 'https://prometheus.invalid.example.com' + + it 'raises a Gitlab::PrometheusError error when a SocketError is rescued' do + req_stub = stub_prometheus_request_with_socket_exception(prometheus_invalid_url) + + expect { subject.send(:get, prometheus_invalid_url) }.to raise_error(Gitlab::PrometheusError, "Can't connect to #{prometheus_invalid_url}") + expect(req_stub).to have_been_requested + end + + it 'raises a Gitlab::PrometheusError error when a SSLError is rescued' do + req_stub = stub_prometheus_request_with_ssl_exception(prometheus_invalid_url) + + expect { subject.send(:get, prometheus_invalid_url) }.to raise_error(Gitlab::PrometheusError, "#{prometheus_invalid_url} contains invalid SSL data") + expect(req_stub).to have_been_requested + end + end + describe '#query' do let(:prometheus_query) { prometheus_cpu_query('env-slug') } let(:query_url) { prometheus_query_url(prometheus_query) } diff --git a/spec/support/prometheus_helpers.rb b/spec/support/prometheus_helpers.rb index cc79b11616a..625a40e1154 100644 --- a/spec/support/prometheus_helpers.rb +++ b/spec/support/prometheus_helpers.rb @@ -33,6 +33,16 @@ module PrometheusHelpers }) end + def stub_prometheus_request_with_socket_exception(url) + WebMock.stub_request(:get, url) + .to_raise(SocketError) + end + + def stub_prometheus_request_with_ssl_exception(url) + WebMock.stub_request(:get, url) + .to_raise(OpenSSL::SSL::SSLError) + end + def stub_all_prometheus_requests(environment_slug, body: nil, status: 200) stub_prometheus_request( prometheus_query_url(prometheus_memory_query(environment_slug)), From 64e811957795293b647bda7aef23ffbd92082614 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Tue, 2 May 2017 16:53:49 -0500 Subject: [PATCH 3/4] Improved code styling and added a HTTParty rescue block --- lib/gitlab/prometheus.rb | 2 ++ spec/lib/gitlab/prometheus_spec.rb | 34 +++++++++++++------ .../prometheus_service_spec.rb | 5 ++- spec/support/prometheus_helpers.rb | 10 ++---- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/lib/gitlab/prometheus.rb b/lib/gitlab/prometheus.rb index 94541009100..aa9810c5c41 100644 --- a/lib/gitlab/prometheus.rb +++ b/lib/gitlab/prometheus.rb @@ -54,6 +54,8 @@ module Gitlab raise PrometheusError, "Can't connect to #{url}" rescue OpenSSL::SSL::SSLError raise PrometheusError, "#{url} contains invalid SSL data" + rescue HTTParty::Error + raise PrometheusError, "An error has ocurred" end def handle_response(response) diff --git a/spec/lib/gitlab/prometheus_spec.rb b/spec/lib/gitlab/prometheus_spec.rb index d8683669518..8e187df6ca4 100644 --- a/spec/lib/gitlab/prometheus_spec.rb +++ b/spec/lib/gitlab/prometheus_spec.rb @@ -49,21 +49,33 @@ describe Gitlab::Prometheus, lib: true do end end - describe 'failure to reach a prometheus url' do - prometheus_invalid_url = 'https://prometheus.invalid.example.com' + describe 'failure to reach a provided prometheus url' do + let(:prometheus_url) {"https://prometheus.invalid.example.com"} - it 'raises a Gitlab::PrometheusError error when a SocketError is rescued' do - req_stub = stub_prometheus_request_with_socket_exception(prometheus_invalid_url) + context 'exceptions are raised' do + it 'raises a Gitlab::PrometheusError error when a SocketError is rescued' do + req_stub = stub_prometheus_request_with_exception(prometheus_url, SocketError) - expect { subject.send(:get, prometheus_invalid_url) }.to raise_error(Gitlab::PrometheusError, "Can't connect to #{prometheus_invalid_url}") - expect(req_stub).to have_been_requested - end + expect { subject.send(:get, prometheus_url) } + .to raise_error(Gitlab::PrometheusError, "Can't connect to #{prometheus_url}") + expect(req_stub).to have_been_requested + end - it 'raises a Gitlab::PrometheusError error when a SSLError is rescued' do - req_stub = stub_prometheus_request_with_ssl_exception(prometheus_invalid_url) + it 'raises a Gitlab::PrometheusError error when a SSLError is rescued' do + req_stub = stub_prometheus_request_with_exception(prometheus_url, OpenSSL::SSL::SSLError) - expect { subject.send(:get, prometheus_invalid_url) }.to raise_error(Gitlab::PrometheusError, "#{prometheus_invalid_url} contains invalid SSL data") - expect(req_stub).to have_been_requested + expect { subject.send(:get, prometheus_url) } + .to raise_error(Gitlab::PrometheusError, "#{prometheus_url} contains invalid SSL data") + expect(req_stub).to have_been_requested + end + + it 'raises a Gitlab::PrometheusError error when a HTTParty::Error is rescued' do + req_stub = stub_prometheus_request_with_exception(prometheus_url, HTTParty::Error) + + expect { subject.send(:get, prometheus_url) } + .to raise_error(Gitlab::PrometheusError, "An error has ocurred") + expect(req_stub).to have_been_requested + end end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 5ef1b53be15..f3126bc1e57 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -93,12 +93,11 @@ describe PrometheusService, models: true, caching: true do [404, 500].each do |status| context "when Prometheus responds with #{status}" do - body_response = 'QUERY_FAILED' before do - stub_all_prometheus_requests(environment.slug, status: status, body: body_response) + stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") end - it { is_expected.to eq(success: false, result: %(#{status} - \"#{body_response}\")) } + it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } end end end diff --git a/spec/support/prometheus_helpers.rb b/spec/support/prometheus_helpers.rb index 625a40e1154..a204365431b 100644 --- a/spec/support/prometheus_helpers.rb +++ b/spec/support/prometheus_helpers.rb @@ -33,14 +33,8 @@ module PrometheusHelpers }) end - def stub_prometheus_request_with_socket_exception(url) - WebMock.stub_request(:get, url) - .to_raise(SocketError) - end - - def stub_prometheus_request_with_ssl_exception(url) - WebMock.stub_request(:get, url) - .to_raise(OpenSSL::SSL::SSLError) + def stub_prometheus_request_with_exception(url, exception_type) + WebMock.stub_request(:get, url).to_raise(exception_type) end def stub_all_prometheus_requests(environment_slug, body: nil, status: 200) From 6eb9e981c61969a904ccbae2c5f52f562b02d199 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Thu, 4 May 2017 09:16:24 -0500 Subject: [PATCH 4/4] Improved changelog entry, also changed error message for HTTParty error --- .../unreleased/prometheus-integration-test-setting-fix.yml | 2 +- lib/gitlab/prometheus.rb | 2 +- spec/lib/gitlab/prometheus_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/changelogs/unreleased/prometheus-integration-test-setting-fix.yml b/changelogs/unreleased/prometheus-integration-test-setting-fix.yml index c65c682a114..45b7c2263e6 100644 --- a/changelogs/unreleased/prometheus-integration-test-setting-fix.yml +++ b/changelogs/unreleased/prometheus-integration-test-setting-fix.yml @@ -1,4 +1,4 @@ --- -title: Added rescue block for the test method +title: Prevent 500 errors caused by testing the Prometheus service merge_request: 10994 author: diff --git a/lib/gitlab/prometheus.rb b/lib/gitlab/prometheus.rb index aa9810c5c41..8827507955d 100644 --- a/lib/gitlab/prometheus.rb +++ b/lib/gitlab/prometheus.rb @@ -55,7 +55,7 @@ module Gitlab rescue OpenSSL::SSL::SSLError raise PrometheusError, "#{url} contains invalid SSL data" rescue HTTParty::Error - raise PrometheusError, "An error has ocurred" + raise PrometheusError, "Network connection error" end def handle_response(response) diff --git a/spec/lib/gitlab/prometheus_spec.rb b/spec/lib/gitlab/prometheus_spec.rb index 8e187df6ca4..fc453a2704b 100644 --- a/spec/lib/gitlab/prometheus_spec.rb +++ b/spec/lib/gitlab/prometheus_spec.rb @@ -73,7 +73,7 @@ describe Gitlab::Prometheus, lib: true do req_stub = stub_prometheus_request_with_exception(prometheus_url, HTTParty::Error) expect { subject.send(:get, prometheus_url) } - .to raise_error(Gitlab::PrometheusError, "An error has ocurred") + .to raise_error(Gitlab::PrometheusError, "Network connection error") expect(req_stub).to have_been_requested end end