TomTom::Matrix#check_for_error_response: Handle HTTP error status codes

We might not always get a nicely formatted JSON
`['error']['description']` response body. Sometimes, like for example
when you use an incorrect API key, even with an 'application/json'
content type, TomTom will respond with:

    <h1>Developer Inactive</h1>

What?

In that case, the response has a 403 status. In addition to checking for
an error in the response, should also be checking for the HTTP status
code.

Log the status code in the exception to give us more information about
what went wrong.

Update our existing tests now that `#check_for_error_response` takes a
response object instead of a JSON string.

Refs #6884

2 files changed, 63 insertions, 20 deletions

diff --git a/lib/tom_tom/matrix.rb b/lib/tom_tom/matrix.rb
index 40191aa56..d0b476d84 100644
--- a/lib/tom_tom/matrix.rb
+++ b/lib/tom_tom/matrix.rb
@@ -24,14 +24,12 @@ module TomTom
         req.body = build_request_body(points)
       end
 
-      matrix_json = JSON.parse(response.body)
-
-      check_for_error_response(matrix_json)
+      check_for_error_response(response)
 
       extract_costs_to_way_costs!(
         way_costs,
         points_with_ids,
-        matrix_json
+        JSON.parse(response.body)
       )
     end
 
@@ -84,9 +82,16 @@ module TomTom
       })
     end
 
-    def check_for_error_response(matrix_json)
-      if matrix_json.has_key?('error')
-        raise RemoteError, matrix_json['error']['description']
+    def check_for_error_response(response)
+      if response.status != 200
+        raise RemoteError, "status: #{response.status}, body: #{response.body}"
+      end
+
+      json = JSON.parse(response.body)
+
+      if json.has_key?('error')
+        raise RemoteError,
+          "status: #{response.status}, message: #{json['error']['description']}"
       end
     end
 
diff --git a/spec/lib/tom_tom/matrix_spec.rb b/spec/lib/tom_tom/matrix_spec.rb
index 7000b56c8..6564b82fd 100644
--- a/spec/lib/tom_tom/matrix_spec.rb
+++ b/spec/lib/tom_tom/matrix_spec.rb
@@ -151,29 +151,67 @@ RSpec.describe TomTom::Matrix do
 
   describe "#check_for_error_response" do
     it "raises a RemoteError when an 'error' key is present in the response" do
-      response_body = {
-        'formatVersion' => '0.0.1',
-        'error' => {
-          'description' => 'Output format: csv is unsupported.'
-        }
-      }
+      response = double(
+        'response',
+        status: 200,
+        body: JSON.dump({
+          'formatVersion' => '0.0.1',
+          'error' => {
+            'description' => 'Output format: csv is unsupported.'
+          }
+        })
+      )
+
+      expect {
+        matrix.check_for_error_response(response)
+      }.to raise_error(
+        TomTom::Matrix::RemoteError,
+        "status: #{response.status}, message: Output format: csv is unsupported."
+      )
+    end
+
+    it "raises a RemoteError when response status is not 200" do
+      response = double(
+        'response',
+        status: 403,
+        body: '<h1>Developer Inactive</h1>'
+      )
 
       expect {
-        matrix.check_for_error_response(response_body)
+        matrix.check_for_error_response(response)
       }.to raise_error(
         TomTom::Matrix::RemoteError,
-        'Output format: csv is unsupported.'
+        "status: #{response.status}, body: <h1>Developer Inactive</h1>"
+      )
+    end
+
+    it "doesn't raise an error when response status is 200" do
+      response = double(
+        'response',
+        status: 200,
+        body: JSON.dump({
+          'formatVersion' => '0.0.1',
+          'matrix' => []
+        })
       )
+
+      expect {
+        matrix.check_for_error_response(response)
+      }.not_to raise_error
     end
 
     it "doesn't raise errors with a normal response" do
-      response_body = {
-        'formatVersion' => '0.0.1',
-        'matrix' => []
-      }
+      response = double(
+        'response',
+        status: 200,
+        body: JSON.dump({
+          'formatVersion' => '0.0.1',
+          'matrix' => []
+        })
+      )
 
       expect {
-        matrix.check_for_error_response(response_body)
+        matrix.check_for_error_response(response)
       }.to_not raise_error
     end
   end