Feature #7596

Find::find should not silently ignores errors

Added by Hoylen Sue over 1 year ago. Updated about 1 month ago.

[ruby-core:51025]
Status:Closed
Priority:Low
Assignee:Yukihiro Matsumoto
Category:lib
Target version:next minor

Description

=begin
The current implementation of (({Find::find})) silently ignores errors. It deliberately catches a number of (({Errno::*})) errors and just continues processing. This can cause unexpected (and often unnoticed) results when, for example, unreadable directories are encountered. Find will not recurse into those directories, but does also not tell the user that it is skipping them and their contents. This happened to me when there was a directory owned by another user.

I suggest making the default behaviour to ((not)) ignore errors. But then provide an ((option)) for the caller to indicate that they want (({find})) to keep going if it encounters some types of errors. This way the caller has control and the default behaviour is the one with "least surprise" for the caller.

Either that, or at least change the documentation to point out that the current implementation silently ignores errors and unreadable directories. That way the caller will know the limitations of the method.

http://www.ruby-doc.org/stdlib-1.9.3/libdoc/find/rdoc/Find.html

When updating the documentation, it would also be useful for documentation for the "Find" module's "find" class method to also point out that:

"The associated block is never called with "." or "..", except when they are explicitly provided as one of the arguments."

You can check this behavour by trying out different values for "testdirname" in the following command

ruby -e 'require "find"; Find.find(ARGV[0]) { |d| puts "<#{d}>" }' testdirname

=end

Associated revisions

Revision 45241
Added by Nobuyoshi Nakada about 1 month ago

find.rb: add ignore_error

  • lib/find.rb (Find#find): add "ignore_error" keyword argument defaulted to true. [Feature #7596]

History

#1 Updated by Yui NARUSE over 1 year ago

  • Tracker changed from Feature to Bug
  • Project changed from CommonRuby to ruby-trunk

#2 Updated by Yui NARUSE about 1 year ago

  • Tracker changed from Bug to Feature

#3 Updated by Eric Hodel about 1 year ago

  • Category set to lib
  • Target version set to next minor

#4 Updated by Koichi Sasada about 1 year ago

  • Assignee set to Yukihiro Matsumoto

I'm not sure who can grab this ticket.
So I assign it to matz.

#5 Updated by Yukihiro Matsumoto 2 months ago

I agree with adding keyword argument, e.g. ignore_error: true.
Choose a good keyword for the option.

Matz.

#6 Updated by Nobuyoshi Nakada 2 months ago

What about an optional error handler?
Simple flag may not be enough, for example, to search unreadable directories.

#7 Updated by Yukihiro Matsumoto about 1 month ago

Nobu, let's not add new things. This time let us focus on stopping ignoring errors.
Probably we can add another keyword argument to specify exception handler.

Matz.

#8 Updated by Nobuyoshi Nakada about 1 month ago

ignore_error: true?

diff --git i/lib/find.rb w/lib/find.rb
index 6f3e428..c5fd35b 100644
--- i/lib/find.rb
+++ w/lib/find.rb
@@ -34,7 +34,7 @@ module Find
   #
   # See the +Find+ module documentation for an example.
   #
-  def find(*paths) # :yield: path
+  def find(*paths, ignore_error: true) # :yield: path
     block_given? or return enum_for(__method__, *paths)

     fs_encoding = Encoding.find("filesystem")
@@ -48,12 +48,14 @@ module Find
           begin
             s = File.lstat(file)
           rescue Errno::ENOENT, Errno::EACCES, Errno::ENOTDIR, Errno::ELOOP, Errno::ENAMETOOLONG
+            raise unless ignore_error
             next
           end
           if s.directory? then
             begin
               fs = Dir.entries(file, encoding: enc)
             rescue Errno::ENOENT, Errno::EACCES, Errno::ENOTDIR, Errno::ELOOP, Errno::ENAMETOOLONG
+              raise unless ignore_error
               next
             end
             fs.sort!
diff --git i/test/test_find.rb w/test/test_find.rb
index b924651..8bd6782 100644
--- i/test/test_find.rb
+++ w/test/test_find.rb
@@ -100,6 +100,16 @@ class TestFind < Test::Unit::TestCase
         a = []
         Find.find(d) {|f| a << f }
         assert_equal([d, dir], a)
+
+        a = []
+        Find.find(d, ignore_error: true) {|f| a << f }
+        assert_equal([d, dir], a)
+
+        a = []
+        assert_raise_with_message(Errno::EACCES, /#{Regexp.quote(dir)}/) do
+          Find.find(d, ignore_error: false) {|f| a << f }
+        end
+        assert_equal([d, dir], a)
       ensure
         File.chmod(0700, dir)
       end
@@ -115,6 +125,17 @@ class TestFind < Test::Unit::TestCase
         a = []
         Find.find(d) {|f| a << f }
         assert_equal([d, dir, file], a)
+
+        a = []
+        Find.find(d, ignore_error: true) {|f| a << f }
+        assert_equal([d, dir, file], a)
+
+        a = []
+        assert_raise_with_message(Errno::EACCES, /#{Regexp.quote(file)}/) do
+          Find.find(d, ignore_error: false) {|f| a << f }
+        end
+        assert_equal([d, dir, file], a)
+
         skip "no meaning test on Windows" if /mswin|mingw/ =~ RUBY_PLATFORM
         assert_raise(Errno::EACCES) { File.lstat(file) }
       ensure

#9 Updated by Nobuyoshi Nakada about 1 month ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r45241.


find.rb: add ignore_error

  • lib/find.rb (Find#find): add "ignore_error" keyword argument defaulted to true. [Feature #7596]

Also available in: Atom PDF