Extend error message in case of HTTP errors in `include`

4 files changed, 65 insertions, 23 deletions

diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb
index 390db997c0b..d02dac66f93 100644
--- a/lib/gitlab/ci/config/external/file/base.rb
+++ b/lib/gitlab/ci/config/external/file/base.rb
@@ -17,9 +17,7 @@ module Gitlab
               @opts = opts
               @errors = []
 
-              validate_location!
-              validate_content!
-              validate_hash!
+              validate!
             end
 
             def invalid_extension?
@@ -46,6 +44,12 @@ module Gitlab
 
             protected
 
+            def validate!
+              validate_location!
+              validate_content! if errors.none?
+              validate_hash! if errors.none?
+            end
+
             def validate_location!
               if invalid_extension?
                 errors.push("Included file `#{location}` does not have YAML extension!")
@@ -53,13 +57,13 @@ module Gitlab
             end
 
             def validate_content!
-              if errors.none? && content.blank?
+              if content.blank?
                 errors.push("Included file `#{location}` is empty or does not exist!")
               end
             end
 
             def validate_hash!
-              if errors.none? && to_hash.blank?
+              if to_hash.blank?
                 errors.push("Included file `#{location}` does not have valid YAML syntax!")
               end
             end
diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb
index acf4201a672..2a256aff65c 100644
--- a/lib/gitlab/ci/config/external/file/local.rb
+++ b/lib/gitlab/ci/config/external/file/local.rb
@@ -24,8 +24,6 @@ module Gitlab
             private
 
             def validate_content!
-              return if errors.any?
-
               if content.nil?
                 errors.push("Local file `#{location}` does not exist!")
               elsif content.blank?
diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb
index b1ba37a3959..23dfc5d9d44 100644
--- a/lib/gitlab/ci/config/external/file/remote.rb
+++ b/lib/gitlab/ci/config/external/file/remote.rb
@@ -23,19 +23,25 @@ module Gitlab
             end
 
             def fetch_remote_content
-              Gitlab::HTTP.get(location)
-            rescue SocketError
-              errors.push("Remote file `#{location}` could not be fetched because of a socket error!")
-              nil
-            rescue Timeout::Error
-              errors.push("Remote file `#{location}` could not be fetched because of a timeout error!")
-              nil
-            rescue Gitlab::HTTP::Error
-              errors.push("Remote file `#{location}` could not be fetched because of a HTTP error!")
-              nil
-            rescue Gitlab::HTTP::BlockedUrlError
-              errors.push("Remote file `#{location}` could not be fetched because the URL is blocked!")
-              nil
+              begin
+                response = Gitlab::HTTP.get(location)
+              rescue SocketError
+                errors.push("Remote file `#{location}` could not be fetched because of a socket error!")
+              rescue Timeout::Error
+                errors.push("Remote file `#{location}` could not be fetched because of a timeout error!")
+              rescue Gitlab::HTTP::Error
+                errors.push("Remote file `#{location}` could not be fetched because of HTTP error!")
+              rescue Gitlab::HTTP::BlockedUrlError
+                errors.push("Remote file `#{location}` could not be fetched because the URL is blocked!")
+              end
+
+              if response&.code.to_i >= 400
+                errors.push <<~ERROR
+                  Remote file `#{location}` could not be fetched because of HTTP code `#{response.code}` error!
+                ERROR
+              end
+
+              response.to_s if errors.none?
             end
           end
         end
diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
index e4682852014..f65a5a1c727 100644
--- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
@@ -105,10 +105,44 @@ describe Gitlab::Ci::Config::External::File::Remote do
   end
 
   describe "#error_message" do
-    let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
+    subject { remote_file.error_message }
 
-    it 'should return an error message' do
-      expect(remote_file.error_message).to eq("Remote file `#{location}` does not have a valid address!")
+    context 'when remote file location is not valid' do
+      let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' }
+
+      it 'returns an error message describing invalid address' do
+        expect(subject).to match /does not have a valid address!/
+      end
+    end
+
+    context 'when timeout error has been raised' do
+      before do
+        WebMock.stub_request(:get, location).to_timeout
+      end
+
+      it 'should returns error message about a timeout' do
+        expect(subject).to match /could not be fetched because of a timeout error!/
+      end
+    end
+
+    context 'when HTTP error has been raised' do
+      before do
+        WebMock.stub_request(:get, location).to_raise(Gitlab::HTTP::Error)
+      end
+
+      it 'should returns error message about a HTTP error' do
+        expect(subject).to match /could not be fetched because of HTTP error!/
+      end
+    end
+
+    context 'when response has 404 status' do
+      before do
+        WebMock.stub_request(:get, location).to_return(body: remote_file_content, status: 404)
+      end
+
+      it 'should returns error message about a timeout' do
+        expect(subject).to match /could not be fetched because of HTTP code `404` error!/
+      end
     end
   end
 end