26 error handling (#39)

* Work on adding custom error handling

* Add error handling to middleware stack

* Work on dynamic error definition

* fix error generation

* Work on ci using middleware

* try different path to get to load in gh ci

* Try adding require to rails helper

* move pre loading

* revert addition to require middleware in rails helper

* Add debuggin steps

* fix typo in debugger action

* try loading manually in application.rb

* try loading in different dir

* fix typo

* fix rubocop errors

* remove debugger actions

* Add spec for different error

* update test title

* Add script to start redis in background and start rails server

* fix rubocop

.rubocop.yml

@@ -37,19 +37,25 @@ Layout/SpaceInsideBlockBraces:
 Lint/UnusedMethodArgument:
   AutoCorrect: false
 Metrics/AbcSize:
+  CountRepeatedAttributes: false
+  Max: 20
   Exclude:
     - "app/models/application_record.rb"
 Metrics/BlockLength:
   Exclude:
     - "spec/**/*.rb"
     - "lib/rubocop/cop/custom/*.rb"
+Metrics/ClassLength:
+  Enabled: false
 Metrics/CyclomaticComplexity:
   Exclude:
     - "lib/rubocop/cop/custom/*.rb"
 Metrics/MethodLength:
+  Max: 20
   Exclude:
     - "lib/rubocop/cop/custom/*.rb"
     - "app/models/application_record.rb"
+    - "app/client/helpers.rb"
 Metrics/PerceivedComplexity:
   Exclude:
     - "lib/rubocop/cop/custom/*.rb"

app/controllers/drivers/selectable_rides_controller.rb

@@ -4,8 +4,8 @@ module Drivers
   class SelectableRidesController < ApplicationController
     def index
       rides = Rides::Commands::RankRides.call(driver:)[offset, limit]
-      opts = { include: %i[from_address to_address] }
-      render json: RidePojoSerializer.new(rides, opts)
+
+      render json: RidePojoSerializer.new(rides, { include: %i[from_address to_address] })
     end
 
     private def driver

app/serializers/api_exception_serializer.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class ApiExceptionSerializer
+  include JSONAPI::Serializer
+  set_type :error
+  set_id do |error, _params|
+    error.status
+  end
+
+  attributes :status, :code, :message
+end

app_start.sh

@@ -0,0 +1,27 @@
+#!/bin/bash
+
+port=3000  # Default port
+
+# Parse command line options
+while getopts ":p:" opt; do
+  case ${opt} in
+    p)
+      port=${OPTARG}
+      ;;
+    \?)
+      echo "Invalid option: -$OPTARG" >&2
+      exit 1
+      ;;
+    :)
+      echo "Option -$OPTARG requires an argument." >&2
+      exit 1
+      ;;
+  esac
+done
+
+# Start Redis in daemon mode
+redis-server --daemonize yes
+
+# Start Rails server on specified port
+echo "Starting Rails server on port $port"
+bundle exec rails server -p $port

config/application.rb

@@ -18,6 +18,8 @@
 # you've limited to :test, :development, or :production.
 Bundler.require(*Rails.groups)
 
+require_relative "../lib/middleware/error_handler"
+
 module RouteRater
   class Application < Rails::Application
     # Initialize configuration defaults for originally generated Rails version.
@@ -28,6 +30,9 @@ class Application < Rails::Application
     # Common ones are `templates`, `generators`, or `middleware`, for example.
     config.autoload_lib(ignore: %w(assets tasks))
     config.autoload_paths << Rails.root.join('lib')
+    config.autoload_paths += %W(#{config.root}/lib)
+
+    config.middleware.use Middleware::ErrorHandler
 
     # Configuration for the application, engines, and railties goes here.
     #

lib/api_exception/base_exception.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module ApiException
+  class BaseException < StandardError
+    include ActiveModel::Serialization
+    attr_reader :status, :code, :message
+
+    def initialize(message, code = 500, status = :internal_error)
+      super(message)
+      @status = status
+      @code = code
+      @message = message
+    end
+  end
+
+  class HTTPRequestError < BaseException; end
+  class JSONParserError < BaseException; end
+  class NoBlockGivenError < BaseException; end
+  class RecordNotFound < BaseException; end
+  class RetryError < BaseException; end
+  class RideCountMismatchError < BaseException; end
+end

lib/api_exception/error_mapper.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module ApiException
+  module ErrorMapper
+    def map_error(exception)
+      custom_error_class_name = "ApiException::#{exception.class.name.split('::').last}"
+      if Object.const_defined?(custom_error_class_name)
+        custom_error_class_name.constantize
+      else
+        define_error_class(custom_error_class_name)
+      end
+    end
+
+    private def define_error_class(class_name)
+      clean_class_name = class_name.split("::").last.gsub(/[^\w]/, "") # Clean up class name
+
+      # Define the error class if it doesn't exist
+      unless Object.const_defined?(clean_class_name)
+        error_class = Class.new(ApiException::BaseException) do
+          def initialize(msg = nil, code = 500, status = :internal_error)
+            super(msg, code, status)
+          end
+        end
+
+        ApiException.const_set(clean_class_name, error_class)
+      end
+
+      ApiException.const_get(clean_class_name)
+    end
+  end
+end

lib/client/helpers.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Client
+  module Helpers
+    def with_retries(max_retries: 10, &blk)
+      raise ApiException::NoBlockGivenError, "Must provide a block" if blk.blank?
+
+      retries = 0
+      begin
+        yield
+      rescue StandardError => _e
+        raise ApiException::RetryError unless retries <= max_retries
+
+        retries += 1
+        max_sleep_seconds = Float(2**retries)
+        sleep rand(0..max_sleep_seconds)
+        retry
+      end
+    end
+  end
+end

lib/middleware/error_handler.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require "./lib/api_exception/error_mapper"
+
+module Middleware
+  class ErrorHandler
+    include ApiException::ErrorMapper # Include the module where map_error and define_error_class are defined
+
+    def initialize(app)
+      @app = app
+    end
+
+    def call(env)
+      @app.call(env)
+    rescue StandardError => e
+      handle_exception(e)
+    end
+
+    private def handle_exception(exception)
+      error_class = map_error(exception)
+      raise error_class, exception.message
+    rescue ApiException::RecordNotFound => e
+      render_error_response(e, 404, :not_found)
+    rescue StandardError => e
+      render_error_response(e, 500, e.status || :internal_error)
+    end
+
+    private def render_error_response(error, status, code)
+      [status, { "Content-Type" => "application/json" },
+       [{ error: { status:, code:, message: error.message } }.to_json]]
+    end
+  end
+end

lib/rides/commands/get_routes_data.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "./lib/api_exception/base_exception"
+
 module Rides
   module Commands
     # Makes a request to the Google API to obtain the route information for all the
@@ -8,6 +10,8 @@ module Commands
     # Even though we only care about one combo per route, it is still more efficient to do it in this manner.
     # We then compare the indexes and get the ones that match, which gives us our original desired routes.
     class GetRoutesData < BaseCommand
+      include Client::Helpers
+
       CACHE = Cache::Store
       DIRECTIONS_API_URL = "https://routes.googleapis.com/distanceMatrix/v2:computeRouteMatrix"
       DEFAULT_HEADERS = {
@@ -16,6 +20,7 @@ class GetRoutesData < BaseCommand
         "Content-Type" => "application/json"
       }.freeze
       DEFAULT_REQUEST_PARAMS = { routingPreference: "TRAFFIC_AWARE", travelMode: "DRIVE" }.freeze
+      MAX_NUM_ELEMENTS = 25
 
       def call(rides:)
         data = get_route_data_for_rides(rides)
@@ -30,7 +35,14 @@ def call(rides:)
         # The response keeps the array positioning on the return. Since we're getting a matrix
         # of routes back, we only want the ones where we explicitly have a 'Ride'. This means that
         # we want the computations where the indicies match.
-        data = data.select { _1[:originIndex] == _1[:destinationIndex] }
+        data = data.flatten.select { _1[:originIndex] == _1[:destinationIndex] }
+        if data.length != rides.length
+          raise ApiException::RideCountMismatchError,
+            "The number of routes does not match the number of rides.",
+            500,
+            :internal_error
+        end
+
         data = transform_keys!(data)
 
         combine_routes_data!(data, rides)
@@ -56,9 +68,12 @@ def call(rides:)
       end
 
       private def get_route_data_for_rides(rides)
-        body = build_request_body(rides)
-        response_body = routes_data(body)
-        JSON.parse(response_body, symbolize_names: true)
+        batches = rides.each_slice(MAX_NUM_ELEMENTS).to_a
+        batches.map do |batch|
+          body = build_request_body(batch)
+          response_body = routes_data(body)
+          JSON.parse(response_body, symbolize_names: true)
+        end
       end
 
       private def routes_data(body)
@@ -85,16 +100,32 @@ def call(rides:)
       end
 
       private def fetch_routes_data(key, body)
-        response = connection.post(
-          DIRECTIONS_API_URL,
-          body.merge(DEFAULT_REQUEST_PARAMS)
-        )
-        body = response.body
-        cache_response!(key, body)
+        response = with_retries do
+          connection.post(
+            DIRECTIONS_API_URL,
+            body.merge(DEFAULT_REQUEST_PARAMS)
+          )
+        end
 
-        body
+        handle_response(response, key)
       end
 
+      private def handle_response(response, key)
+        if response.status != 200
+          result = JSON.parse(response.body, symbolize_names: true)
+          error = result.first[:error]
+          raise ApiException::HTTPRequestError.new(error[:message], error[:status].downcase.to_sym, error[:code])
+        else
+          body = response.body
+          cache_response!(key, body)
+
+          body
+        end
+      rescue JSON::ParserError => e
+        message = e.message
+        Rails.logger.warn "Attemped to parse invalid JSON: #{message}"
+        raise ApiException::JSONParserError, message
+      end
       private def cache_response!(key, value)
         CACHE.set(key, value)
       end

lib/rides/commands/rank_rides.rb

@@ -42,9 +42,11 @@ def call(driver:)
         rides = route_data(driver)
 
         if commutes.count != rides.count
-          raise RideCountMismatchError,
+          raise ApiException::RideCountMismatchError,
             "The number of rides doesn't match the number of commute rides." \
-            "Please check the ride(s) configuration and try again."
+            "Please check the ride(s) configuration and try again.",
+            500,
+            :internal_error
         end
 
         combine_rides!(rides, commutes)

spec/errors/error_spec.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe "Errors", :skip_geocode, type: :request do
+  it "raises JSONParserError and responds with correct error shape" do
+    expect(Rides::Commands::GetRoutesData).to receive(:call).and_raise(
+      ApiException::JSONParserError.new("Attemped to parse invalid JSON:"), 500
+    )
+    driver = create(:driver)
+    get "/drivers/#{driver.id}/selectable_rides"
+
+    expect(response).to have_http_status(500)
+    result = JSON.parse(response.body, symbolize_names: true)
+
+    expect(result[:error].keys).to include(:status, :code, :message)
+    expect(result.dig(:error, :message)).to eq("Attemped to parse invalid JSON:")
+    expect(result.dig(:error, :code)).to eq("internal_error")
+  end
+end

spec/rails_helper.rb

@@ -2,9 +2,11 @@
 
 # This file is copied to spec/ when you run 'rails generate rspec:install'
 ENV["RAILS_ENV"] ||= "test"
+
 require_relative "../config/environment"
 # Prevent database truncation if the environment is production
 abort("The Rails environment is running in production mode!") if Rails.env.production?
+
 require "rspec/rails"
 require "spec_helper"
 require "webmock/rspec"

spec/requests/drivers/selectable_rides_spec.rb

@@ -3,6 +3,17 @@
 require "rails_helper"
 
 RSpec.describe "Drivers::Rides", type: :request do
+  describe "errors" do
+    it "raises NotFoundError when unable to find driver" do
+      get "/drivers/1/selectable_rides"
+      expect(response).to have_http_status(:not_found)
+
+      result = JSON.parse(response.body, symbolize_names: true)
+      expect(result[:error].keys).to include(:status, :code, :message)
+      expect(result.dig(:error, :message)).to eq("Couldn't find Driver with 'id'=1")
+    end
+  end
+
   describe "GET /drivers/:driver_id/rides" do
     it "returns ranked rides" do
       VCR.use_cassette("ranked_rides") do