fix: #3 handle domains without A-record

If a domain only has an mx record but no A record it
will trigger a ConnectionError when attempting the
nicknym key lookup.

We need to detect and handle this in the http adapter
already because once the exception is handled by Celluloid
our actor will be terminated.

So now we allow for handing a rescue option to the
adapter with a string that is checked for inclusion in
the error message. If the string is found the exception
will be caught and the adapter returns nil.

We only make use of this when checking the availability
of nicknym so far. That should be the only http request
going out.

lib/nickserver/adapters/http.rb

@@ -2,6 +2,19 @@ require 'nickserver/adapters'
 require 'nickserver/config'
 require 'http'
 
+# Nickserver::Adapters::Http
+#
+# Basic http adapter with ssl and minimal error handling.
+# Only implemented get requests so far.
+#
+# Error Handling:
+#
+# Pass a string as the 'rescue' option. If a ConnectionError occures
+# which includes the string passed it will be rescued and the request
+# will return nil. This allows handling the error inside the adapter so
+# that for the derived CelluloidHttp Adapter the actor does not get
+# killed.
+
 module Nickserver::Adapters
   class Http
 
@@ -9,6 +22,8 @@ module Nickserver::Adapters
       url = HTTP::URI.parse url.to_s
       response = get_with_auth url, params: options[:query]
       return response.code, response.to_s
+    rescue HTTP::ConnectionError => e
+      raise unless options[:rescue] && e.to_s.include?(options[:rescue])
     end
 
     protected

lib/nickserver/nicknym/source.rb

@@ -8,7 +8,8 @@ module Nickserver
       PORT = 6425
 
       def available_for?(domain)
-        status, body = adapter.get "https://#{domain}/provider.json"
+        status, body = adapter.get "https://#{domain}/provider.json",
+          rescue: 'failed to connect: getaddrinfo'
         status == 200 && provider_with_mx?(body)
       end
 

test/functional/sample_test.rb

@@ -24,6 +24,11 @@ class SampleTest < FunctionalTest
 
   # Regression Tests
 
+  # #3 handle missing A records
+  def test_nicknym
+    assert_lookup_status 404, 'postmaster@cs.ucl.ac.uk'
+  end
+
   def test_no_file_descriptors_leak
     lookup 'test@mail.bitmask.net'
     before = open_files_count

test/remote/Readme.md

 Integration tests for clients of remote services
 ================================================
 
-The tests in this directory are integration test with remote services.  However
-we aims at testing the client side of the equation as that is what we control
-here.
+The tests in this directory are integration test with remote services.
+However we aims at testing the client side of the equation as that is
+what we control here.
 
-So unexpected server behavious should *crash* the test if we are not dealing
-with it properly yet and have no unit test for it.
+So unexpected server behavious should *crash* the test if we are not
+dealing with it properly yet and have no unit test for it.
 
-Server responses that we do not expect but handle in the code and test in unit
-tests make the test *skip*.
+Server responses that we do not expect but handle in the code and test
+in unit tests make the test *skip*.
 
 The Behaviour we would normally expect should make the test *pass*
-

test/remote/nicknym_source_test.rb

@@ -10,13 +10,24 @@ require 'nickserver/email_address'
 class RemoteNicknymSourceTest < CelluloidTest
   include HttpAdapterHelper
 
-  def test_availablility_check
+  def test_available_for_mail
     source.available_for? 'mail.bitmask.net'
-    refute source.available_for? 'dl.bitmask.net'   # not a provider
   rescue HTTP::ConnectionError => e
     skip e.to_s
   end
 
+  # not a provider
+  def test_not_available
+    refute source.available_for? 'dl.bitmask.net'
+  rescue HTTP::ConnectionError => e
+    skip e.to_s
+  end
+
+  # cs.ucl.ac.uk only has an MX not an A-record
+  def test_not_available_without_a_record
+    refute source.available_for? 'cs.ucl.ac.uk'
+  end
+
   def test_successful_query
     response = source.query(email_with_key)
     skip if response.status == 404

test/support/http_stub_helper.rb

@@ -10,32 +10,31 @@ module HttpStubHelper
   end
 
   def stub_nicknym_available_response(domain, response = {})
-    stub_http_request :get, "https://#{domain}/provider.json",
-      response: response
+    stub_http_get "https://#{domain}/provider.json",
+      response,
+      Hash
   end
 
   def stub_sks_vindex_reponse(uid, response = {})
-    stub_http_request :get, config.hkp_url,
-      query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'},
-      response: response
+    stub_http_get config.hkp_url, response,
+      query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'}
   end
 
   def stub_sks_get_reponse(key_id, response = {})
-    stub_http_request :get, config.hkp_url,
-      query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'},
-      response: response
+    stub_http_get config.hkp_url, response,
+      query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'}
   end
 
   def stub_couch_response(uid, response = {})
     query = "\?key=#{"%22#{uid}%22"}&reduce=false"
-    stub_http_request :get,
-      /#{Regexp.escape(config.couch_url)}.*#{query}/,
-      response: response
+    stub_http_get /#{Regexp.escape(config.couch_url)}.*#{query}/,
+      response
   end
 
-  def stub_http_request(verb, url, options = {})
-    response = {status: 200, body: ""}.merge(options.delete(:response) || {})
-    options = nil if options == {}
+  private
+
+  def stub_http_get(url, response, options = nil)
+    response = {status: 200, body: ""}.merge(response || {})
     adapter.expect :get, [response[:status], response[:body]],
       [url, options].compact
   end

test/unit/nicknym/source_test.rb

 require 'test_helper'
+require 'http'
+require 'json'
 require 'nickserver/nicknym/source'
 require 'nickserver/email_address'
 
@@ -20,6 +22,11 @@ class NicknymSourceTest < Minitest::Test
     refute available_on?(200, 'blablabla')
   end
 
+  # adapter rescues name resolution errors and returns nothing
+  def test_not_available_without_response
+    refute available_on?
+  end
+
   def test_proxy_successful_query
     assert proxies_query_response?(200, 'dummy body')
   end
@@ -39,9 +46,9 @@ class NicknymSourceTest < Minitest::Test
     adapter.verify
   end
 
-  def available_on?(status = 0, body = nil)
-    adapter.expect :get, [status, body],
-      ['https://remote.tld/provider.json']
+  def available_on?(*args)
+    adapter.expect :get, args,
+      ['https://remote.tld/provider.json', Hash]
     available = source.available_for?('remote.tld')
     adapter.verify
     return available