health: catch all exceptions for registries

The fact that `RegistryClient.reachable?` might raise an exception is
fine in some situations on the API, but it prevents us from knowing from
other errors, and it also makes things harder in other places of the
code that rely on this check. Hence, let's rescue this exception and
return its value properly.

Signed-off-by: Miquel Sabaté Solà <[email protected]>

lib/portus/health_checks/registry.rb

@@ -15,8 +15,8 @@ def self.ready
 
         res = ::Registry.get.client.reachable?
         ["registry is#{res ? "" : " not"} reachable", res]
-
-        # TODO: rescue ::Portus::RequestError instead of simply bailing
+      rescue ::Portus::RequestError => e
+        [e.to_s, false]
       end
     end
   end

lib/portus/request_error.rb

@@ -8,7 +8,7 @@ class RequestError < StandardError
     # Given an inner exception and a message, it builds up a common error
     # message.
     def initialize(exception:, message:)
-      @msg = "#{exception.class.name}: #{message}."
+      @msg = "#{exception.class.name}: #{message}"
       Rails.logger.error @msg
     end
 

spec/api/grape_api/v1/health_spec.rb

@@ -28,6 +28,18 @@ def db_helper_msg
         expect(response.status).to eq 503
       end
 
+      it "reached a RequestError when trying to connect to the registry" do
+        create(:registry, hostname: "whatever", use_ssl: false)
+
+        allow_any_instance_of(::Portus::RegistryClient).to receive(:reachable?)
+          .and_raise(::Portus::RequestError, exception: Net::ReadTimeout, message: "something")
+
+        get "/api/v1/health"
+        expect(response.status).to eq 503
+        data = JSON.parse(response.body)
+        expect(data["registry"]["msg"]).to eq "Class: something"
+      end
+
       it "has both the DB and the registry" do
         create(:registry, hostname: "registry.mssola.cat", use_ssl: true)
 

spec/integration/health.bats

@@ -0,0 +1,23 @@
+#!/usr/bin/env bats -t
+
+load helpers
+
+function setup() {
+    __setup minimal
+}
+
+@test "health runs just fine" {
+    helper_runner curl.rb /api/v1/health
+    [[ "${lines[-2]}" =~ "database is up-to-date" ]]
+    [[ "${lines[-2]}" =~ "clair is reachable" ]]
+    [[ "${lines[-2]}" =~ "registry is reachable" ]]
+}
+
+@test "health reports an invalid registry" {
+    # Modify the registry hostname to some unknown hostname.
+    ruby_puts "Registry.get.update(hostname:\"wrong.whatever\")"
+
+    helper_runner curl.rb /api/v1/health
+    [ $status -eq 1 ]
+    [[ "${lines[-2]}" =~ "SocketError: connection refused" ]]
+}

spec/lib/portus/request_error_spec.rb

@@ -6,6 +6,6 @@
 
     expect do
       raise ::Portus::RequestError.new(exception: e, message: "something")
-    end.to raise_error(::Portus::RequestError, "StandardError: something.")
+    end.to raise_error(::Portus::RequestError, "StandardError: something")
   end
 end