Merge pull request #553 from af83/6884-tomtom-matrix--handle-error-when-response-doesn,t-inclu

Handle API error(s) in Tomtom matrix. Fixes #6884

6 files changed, 123 insertions, 0 deletions

diff --git a/app/services/route_way_cost_calculator.rb b/app/services/route_way_cost_calculator.rb
index d41a2e59a..ca47a6772 100644
--- a/app/services/route_way_cost_calculator.rb
+++ b/app/services/route_way_cost_calculator.rb
@@ -8,5 +8,7 @@ class RouteWayCostCalculator
     way_costs = TomTom.matrix(way_costs)
     way_costs = WayCostCollectionJSONSerializer.dump(way_costs)
     @route.update(costs: way_costs)
+  rescue TomTom::Errors::MatrixRemoteError => e
+    Rails.logger.error "TomTom::Matrix server error: #{e}"
   end
 end
diff --git a/lib/tom_tom/errors.rb b/lib/tom_tom/errors.rb
new file mode 100644
index 000000000..da3f2239c
--- /dev/null
+++ b/lib/tom_tom/errors.rb
@@ -0,0 +1,4 @@
+module TomTom
+  module Errors
+  end
+end
diff --git a/lib/tom_tom/errors/matrix_remote_error.rb b/lib/tom_tom/errors/matrix_remote_error.rb
new file mode 100644
index 000000000..b13767847
--- /dev/null
+++ b/lib/tom_tom/errors/matrix_remote_error.rb
@@ -0,0 +1,5 @@
+module TomTom
+  module Errors
+    class MatrixRemoteError < RuntimeError; end
+  end
+end
diff --git a/lib/tom_tom/matrix.rb b/lib/tom_tom/matrix.rb
index c418cd516..e779abbf7 100644
--- a/lib/tom_tom/matrix.rb
+++ b/lib/tom_tom/matrix.rb
@@ -4,6 +4,10 @@ module TomTom
       @connection = connection
     end
 
+    # Exceptions:
+    #
+    # * This raises a `TomTom::Errors::MatrixRemoteError` when the API responds
+    #   with an error.
     def matrix(way_costs)
       points_with_ids = points_from_way_costs(way_costs)
       points = points_as_params(points_with_ids)
@@ -21,6 +25,8 @@ module TomTom
         req.body = build_request_body(points)
       end
 
+      check_for_error_response(response)
+
       extract_costs_to_way_costs!(
         way_costs,
         points_with_ids,
@@ -77,6 +83,20 @@ module TomTom
       })
     end
 
+    def check_for_error_response(response)
+      if response.status != 200
+        raise TomTom::Errors::MatrixRemoteError,
+          "status: #{response.status}, body: #{response.body}"
+      end
+
+      json = JSON.parse(response.body)
+
+      if json.has_key?('error')
+        raise TomTom::Errors::MatrixRemoteError,
+          "status: #{response.status}, message: #{json['error']['description']}"
+      end
+    end
+
     def extract_costs_to_way_costs!(way_costs, points, matrix_json)
       way_costs = []
 
diff --git a/spec/lib/tom_tom/matrix_spec.rb b/spec/lib/tom_tom/matrix_spec.rb
index 605f1d254..f914cf7ff 100644
--- a/spec/lib/tom_tom/matrix_spec.rb
+++ b/spec/lib/tom_tom/matrix_spec.rb
@@ -149,6 +149,73 @@ RSpec.describe TomTom::Matrix do
     end
   end
 
+  describe "#check_for_error_response" do
+    it "raises a MatrixRemoteError when an 'error' key is present in the response" do
+      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::Errors::MatrixRemoteError,
+        "status: #{response.status}, message: Output format: csv is unsupported."
+      )
+    end
+
+    it "raises a MatrixRemoteError when response status is not 200" do
+      response = double(
+        'response',
+        status: 403,
+        body: '<h1>Developer Inactive</h1>'
+      )
+
+      expect {
+        matrix.check_for_error_response(response)
+      }.to raise_error(
+        TomTom::Errors::MatrixRemoteError,
+        "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 = double(
+        'response',
+        status: 200,
+        body: JSON.dump({
+          'formatVersion' => '0.0.1',
+          'matrix' => []
+        })
+      )
+
+      expect {
+        matrix.check_for_error_response(response)
+      }.to_not raise_error
+    end
+  end
+
   describe "#extract_costs_to_way_costs!" do
     it "puts distance & time costs in way_costs" do
       way_costs = [
diff --git a/spec/services/route_way_cost_calculator_spec.rb b/spec/services/route_way_cost_calculator_spec.rb
index d11db2950..aa6195196 100644
--- a/spec/services/route_way_cost_calculator_spec.rb
+++ b/spec/services/route_way_cost_calculator_spec.rb
@@ -31,5 +31,30 @@ RSpec.describe RouteWayCostCalculator do
       expect(route.costs).not_to be_nil
       expect { JSON.parse(JSON.dump(route.costs)) }.not_to raise_error
     end
+
+    it "doesn't update route costs when there is a server error" do
+      route = create(:route)
+
+      stub_request(
+        :post,
+        "https://api.tomtom.com/routing/1/matrix/json?key&routeType=shortest&travelMode=bus"
+      )
+        .with(
+          headers: {
+            'Accept'=>'*/*',
+            'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
+            'Content-Type'=>'application/json',
+            'User-Agent'=>'Faraday v0.9.2'
+          })
+        .to_return(
+          status: 200,
+          body: "{\"formatVersion\":\"0.0.1\",\"error\":{\"description\":\"Outputformat:csvisunsupported.\"}}",
+          headers: {}
+        )
+
+      RouteWayCostCalculator.new(route).calculate!
+
+      expect(route.costs).to be_nil
+    end
   end
 end