Improve exception for #policy_scope and #policy_scope! (#548)

This changes avoid to raise an InvalidConstructorError if we catch an
ArgumentError exception from #resolve method of Policy Scope.

lib/pundit.rb

@@ -80,10 +80,16 @@ module Pundit
     # @raise [InvalidConstructorError] if the policy constructor called incorrectly
     # @return [Scope{#resolve}, nil] instance of scope class which can resolve to a scope
     def policy_scope(user, scope)
-      policy_scope = PolicyFinder.new(scope).scope
-      policy_scope.new(user, pundit_model(scope)).resolve if policy_scope
-    rescue ArgumentError
-      raise InvalidConstructorError, "Invalid #<#{policy_scope}> constructor is called"
+      policy_scope_class = PolicyFinder.new(scope).scope
+      return unless policy_scope_class
+
+      begin
+        policy_scope = policy_scope_class.new(user, pundit_model(scope))
+      rescue ArgumentError
+        raise InvalidConstructorError, "Invalid #<#{policy_scope_class}> constructor is called"
+      end
+
+      policy_scope.resolve
     end
 
     # Retrieves the policy scope for the given record.
@@ -95,10 +101,16 @@ module Pundit
     # @raise [InvalidConstructorError] if the policy constructor called incorrectly
     # @return [Scope{#resolve}] instance of scope class which can resolve to a scope
     def policy_scope!(user, scope)
-      policy_scope = PolicyFinder.new(scope).scope!
-      policy_scope.new(user, pundit_model(scope)).resolve
-    rescue ArgumentError
-      raise InvalidConstructorError, "Invalid #<#{policy_scope}> constructor is called"
+      policy_scope_class = PolicyFinder.new(scope).scope!
+      return unless policy_scope_class
+
+      begin
+        policy_scope = policy_scope_class.new(user, pundit_model(scope))
+      rescue ArgumentError
+        raise InvalidConstructorError, "Invalid #<#{policy_scope_class}> constructor is called"
+      end
+
+      policy_scope.resolve
     end
 
     # Retrieves the policy for the given record.

spec/pundit_spec.rb

@@ -16,6 +16,7 @@ describe Pundit do
   let(:tag_four_five_six) { ProjectOneTwoThree::TagFourFiveSix.new(user) }
   let(:avatar_four_five_six) { ProjectOneTwoThree::AvatarFourFiveSix.new }
   let(:wiki) { Wiki.new }
+  let(:thread) { Thread.new }
 
   describe ".authorize" do
     it "infers the policy and authorizes based on it" do
@@ -88,6 +89,12 @@ describe Pundit do
         Pundit.policy_scope(user, Wiki)
       end.to raise_error(Pundit::InvalidConstructorError, "Invalid #<WikiPolicy::Scope> constructor is called")
     end
+
+    it "raises an original error with a policy scope that contains error" do
+      expect do
+        Pundit.policy_scope(user, Thread)
+      end.to raise_error(ArgumentError)
+    end
   end
 
   describe ".policy_scope!" do

spec/spec_helper.rb

@@ -236,6 +236,18 @@ class WikiPolicy
   end
 end
 
+class Thread
+  def self.all; end
+end
+class ThreadPolicy < Struct.new(:user, :thread)
+  class Scope < Struct.new(:user, :scope)
+    def resolve
+      # deliberate wrong useage of the method
+      scope.all(:unvalid, :parameters)
+    end
+  end
+end
+
 class PostFourFiveSix < Struct.new(:user); end
 
 class CommentFourFiveSix; extend ActiveModel::Naming; end