Merge pull request #92 from rage-rb/argument-errors

Update error classes

lib/rage/configuration.rb

@@ -114,7 +114,7 @@ class Rage::Configuration
   def config = self
 
   def log_formatter=(formatter)
-    raise "Custom log formatter should respond to `#call`" unless formatter.respond_to?(:call)
+    raise ArgumentError, "Custom log formatter should respond to `#call`" unless formatter.respond_to?(:call)
     @log_formatter = formatter
   end
 

lib/rage/controller/api.rb

@@ -168,7 +168,7 @@ def rescue_from(*klasses, with: nil, &block)
         if block_given?
           with = define_tmp_method(block)
         else
-          raise "No handler provided. Pass the `with` keyword argument or provide a block."
+          raise ArgumentError, "No handler provided. Pass the `with` keyword argument or provide a block."
         end
       end
 
@@ -224,7 +224,7 @@ def before_action(action_name = nil, **opts, &block)
 
     # Register a new `after_action` hook. Calls with the same `action_name` will overwrite the previous ones.
     #
-    # @param action_name [String, nil] the name of the callback to add
+    # @param action_name [Symbol, nil] the name of the callback to add
     # @param [Hash] opts action options
     # @option opts [Symbol, Array<Symbol>] :only restrict the callback to run only for specific actions
     # @option opts [Symbol, Array<Symbol>] :except restrict the callback to run for all actions except specified
@@ -250,14 +250,14 @@ def after_action(action_name = nil, **opts, &block)
 
     # Prevent a `before_action` hook from running.
     #
-    # @param action_name [String] the name of the callback to skip
+    # @param action_name [Symbol] the name of the callback to skip
     # @param only [Symbol, Array<Symbol>] restrict the callback to be skipped only for specific actions
     # @param except [Symbol, Array<Symbol>] restrict the callback to be skipped for all actions except specified
     # @example
     #   skip_before_action :find_photo, only: :create
     def skip_before_action(action_name, only: nil, except: nil)
       i = @__before_actions&.find_index { |a| a[:name] == action_name }
-      raise "The following action was specified to be skipped but couldn't be found: #{self}##{action_name}" unless i
+      raise ArgumentError, "The following action was specified to be skipped but couldn't be found: #{self}##{action_name}" unless i
 
       @__before_actions = @__before_actions.dup if @__before_actions.frozen?
 
@@ -284,7 +284,7 @@ def prepare_action_params(action_name = nil, **opts, &block)
       if block_given?
         action_name = define_tmp_method(block)
       elsif action_name.nil?
-        raise "No handler provided. Pass the `action_name` parameter or provide a block."
+        raise ArgumentError, "No handler provided. Pass the `action_name` parameter or provide a block."
       end
 
        _only, _except, _if, _unless = opts.values_at(:only, :except, :if, :unless)

lib/rage/middleware/cors.rb

@@ -64,7 +64,7 @@ def allow(*origins, methods: "*", allow_headers: "*", expose_headers: nil, max_a
     @methods = if methods != "*"
       methods.map! { |method| method.to_s.upcase }.tap { |m|
         if (invalid_methods = m - @default_methods).any?
-          raise "Unsupported method passed to Rage::Cors: #{invalid_methods[0]}"
+          raise ArgumentError, "Unsupported method passed to Rage::Cors: #{invalid_methods[0]}"
         end
       }.join(", ")
     elsif @allow_credentials
@@ -74,8 +74,8 @@ def allow(*origins, methods: "*", allow_headers: "*", expose_headers: nil, max_a
     end
 
     if @allow_credentials
-      raise "Rage::Cors requires you to explicitly list allowed headers when using `allow_credentials: true`" if @allow_headers == "*"
-      raise "Rage::Cors requires you to explicitly list exposed headers when using `allow_credentials: true`" if @expose_headers == "*"
+      raise ArgumentError, "Rage::Cors requires you to explicitly list allowed headers when using `allow_credentials: true`" if @allow_headers == "*"
+      raise ArgumentError, "Rage::Cors requires you to explicitly list exposed headers when using `allow_credentials: true`" if @expose_headers == "*"
     end
 
     @origins = []

lib/rage/router/backend.rb

@@ -20,7 +20,7 @@ def reset_routes
   end
 
   def mount(path, handler, methods)
-    raise "Mount handler should respond to `call`" unless handler.respond_to?(:call)
+    raise ArgumentError, "Mount handler should respond to `call`" unless handler.respond_to?(:call)
 
     raw_handler = handler
     is_sidekiq = handler.respond_to?(:name) && handler.name == "Sidekiq::Web"
@@ -52,7 +52,7 @@ def on(method, path, handler, constraints: {}, defaults: nil)
     raise "Path could not be empty" if path&.empty?
 
     if match_index = (path =~ OPTIONAL_PARAM_REGEXP)
-      raise "Optional Parameter has to be the last parameter of the path" if path.length != match_index + $&.length
+      raise ArgumentError, "Optional Parameter has to be the last parameter of the path" if path.length != match_index + $&.length
 
       path_full = path.sub(OPTIONAL_PARAM_REGEXP, "/#{$1}")
       path_optional = path.sub(OPTIONAL_PARAM_REGEXP, "")
@@ -65,7 +65,7 @@ def on(method, path, handler, constraints: {}, defaults: nil)
     meta = { raw_handler: handler }
 
     if handler.is_a?(String)
-      raise "Invalid route handler format, expected to match the 'controller#action' pattern" unless handler =~ STRING_HANDLER_REGEXP
+      raise ArgumentError, "Invalid route handler format, expected to match the 'controller#action' pattern" unless handler =~ STRING_HANDLER_REGEXP
 
       controller, action = Rage::Router::Util.path_to_class($1), $2
 
@@ -81,7 +81,7 @@ def on(method, path, handler, constraints: {}, defaults: nil)
         handler = ->(_, _) { [404, { "X-Cascade" => "pass" }, []] }
       end
     else
-      raise "Non-string route handler should respond to `call`" unless handler.respond_to?(:call)
+      raise ArgumentError, "Non-string route handler should respond to `call`" unless handler.respond_to?(:call)
       # while regular handlers are expected to be called with the `env` and `params` objects,
       # lambda handlers expect just `env` as an argument;
       # TODO: come up with something nicer?
@@ -189,7 +189,7 @@ def __on(method, path, handler, constraints, defaults, meta)
         params << "*"
         current_node = current_node.create_wildcard_child
         parent_node_path_index = i + 1
-        raise "Wildcard must be the last character in the route" if i != pattern.length - 1
+        raise ArgumentError, "Wildcard must be the last character in the route" if i != pattern.length - 1
       end
 
       i += 1
@@ -205,7 +205,7 @@ def __on(method, path, handler, constraints, defaults, meta)
         existing_route[:pattern] == pattern &&
         existing_route[:constraints] == constraints
       )
-        raise "Method '#{method}' already declared for route '#{pattern}' with constraints '#{constraints.inspect}'"
+        raise ArgumentError, "Method '#{method}' already declared for route '#{pattern}' with constraints '#{constraints.inspect}'"
       end
     end
 

lib/rage/router/constrainer.rb

@@ -45,14 +45,14 @@ def note_usage(constraints)
   end
 
   def new_store_for_constraint(constraint)
-    raise "No strategy registered for constraint key '#{constraint}'" unless @strategies[constraint]
+    raise ArgumentError, "No strategy registered for constraint key '#{constraint}'" unless @strategies[constraint]
     @strategies[constraint].storage
   end
 
   def validate_constraints(constraints)
     constraints.each do |key, value|
       strategy = @strategies[key]
-      raise "No strategy registered for constraint key '#{key}'" unless strategy
+      raise ArgumentError, "No strategy registered for constraint key '#{key}'" unless strategy
 
       strategy.validate(value)
     end
@@ -73,7 +73,7 @@ def __build_derive_constraints
         if key == :host
           lines << "   host: env['HTTP_HOST'.freeze],"
         else
-          raise 'unknown non-custom strategy for compiling constraint derivation function'
+          raise ArgumentError, 'unknown non-custom strategy for compiling constraint derivation function'
         end
       else
         lines << "  #{strategy.name}: @strategies[#{key}].derive_constraint(env),"

lib/rage/router/dsl.rb

@@ -325,7 +325,7 @@ def resources(*_resources, **opts, &block)
       end
 
       _module, _path, _only, _except, _param = opts.values_at(:module, :path, :only, :except, :param)
-      raise ":param option can't contain colons" if _param.to_s.include?(":")
+      raise ArgumentError, ":param option can't contain colons" if _param.to_s.include?(":")
 
       _only = Array(_only) if _only
       _except = Array(_except) if _except
@@ -395,7 +395,7 @@ def __on(method, path, to, constraints, defaults)
         if @controllers.any?
           to = "#{@controllers.last}##{path}"
         else
-          raise "Missing :to key on routes definition, please check your routes."
+          raise ArgumentError, "Missing :to key on routes definition, please check your routes."
         end
       end
 

lib/rage/router/handler_storage.rb

@@ -35,7 +35,7 @@ def add_handler(constrainer, route)
     end
 
     if @handlers.length >= 32
-      raise "Limit reached: a maximum of 32 route handlers per node allowed when there are constraints"
+      raise ArgumentError, "Limit reached: a maximum of 32 route handlers per node allowed when there are constraints"
     end
 
     @handlers << handler_object

lib/rage/router/strategies/host.rb

@@ -18,7 +18,7 @@ def custom?
 
   def validate(value)
     if !value.is_a?(String) && !value.is_a?(Regexp)
-      raise "Host should be a string or a Regexp"
+      raise ArgumentError, "Host should be a string or a Regexp"
     end
   end