Repository: sharkdp/fd (language: rust)

Issue:
Issue error messages (eg permission denied)
If `fd` can't search a directory, there is no notification:

```
% mkdir -m000 noperms
% fd -HI moo noperms
%
```

I'd expect a `permission denied` by default.

If not default (why not? that's what STDERR is for), perhaps add a switch to enable them.


Discussion / hints:
Thank you for the feedback.

I agree. We should definitely make a plan on how to deal with such errors.

I'm not sure if we should always show them by default, but I agree that we should definitely show an error message if one of the explicitly given search directories can not be traversed.
What if the errors were shown by default, but there was a flag to suppress them? 
This way the user could still be alerted when an error occurred. On the other hand, the flag could be used in an automation perspective, like piping output to other tools.
STDOUT (File descriptor 1) gets piped to the next command if there is a pipe.

For errors, there's STDERR (FD 2) which gets connected to the terminal by default, but can be redirected to `/dev/null` (or anywhere else):

`command 2> /dev/null`

[Similar behaviour in Windows](https://support.microsoft.com/en-us/help/110930/redirecting-error-messages-from-command-prompt-stderr-stdout)


This is standard Unix design philosophy, where MS got it from. Not having errors enabled by default will raise many eyebrows without a solid reason to deviate from established practice.

Not showing errors by default means that possibly matching files which can't be accessed due to permissions issues will never be highlighted.
Hi, I am new to Rust and wanted to gain some experience by contributing to the open source community. Is this issue still there? If yes, could you give me some pointers on how to proceed with the issue? Thanks!
Thanks for wanting to help out!

I'm not a rust man, but this does look like a good starting project.

The issue persists. 

I suggest you tag some of the others above for guidance - I can't do that this Fasthub app.
@sharkdp Could you help me in proceeding with this issue? Any tips would be helpful! 
@psinghal20 Thank you for offering to help out!

A quick disclaimer: I did initially put the "good first issue" label on this ticket, but I now realize that it's probably not the most beginner-friendly thing to work on.

That being said, a good place to start would be this `match` statement:
https://github.com/sharkdp/fd/blob/27caa337296398d115f8779e08606897d8d9ad5e/src/walk.rs#L207-L210
where we currently silently ignore any errors by skipping to the next entry (`return ignore::WalkState::Continue`).

A really quick & dirty solution would be to add a print-statement here before returning. However, note that this part of the code runs in a parallelized way inside the "walker" threads (that do the actual work of finding files). Printing unsynchronized from these threads could potentially result in a messed-up terminal output.

A better way to fix this would be to send errors via the `mpsc::channel` that these worker threads use to communicate with the main thread. Currently, we use the channel to send `PathBuf` objects for every entry that `fd` finds. To extend this in such a way that we could alternatively send errors, we would probably write a new `enum` type along the lines of
``` rust
enum WorkerResult {
  Entry(PathBuf),
  Error(SomeErrorType)
}
```
that could then by used as the underlying type for the `mpsc::channel`.

On the receiving side of the channel, we could then print the errors in a synchronized way from a single thread.

Fix patch:
diff --git a/src/exec/job.rs b/src/exec/job.rs
index 3b0fa9cc2..cb33ab836 100644
--- a/src/exec/job.rs
+++ b/src/exec/job.rs
@@ -6,24 +6,33 @@
 // notice may not be copied, modified, or distributed except
 // according to those terms.
 
+use super::CommandTemplate;
+use internal::print_error;
 use std::path::PathBuf;
 use std::sync::mpsc::Receiver;
 use std::sync::{Arc, Mutex};
-
-use super::CommandTemplate;
+use walk::WorkerResult;
 
 /// An event loop that listens for inputs from the `rx` receiver. Each received input will
 /// generate a command with the supplied command template. The generated command will then
 /// be executed, and this process will continue until the receiver's sender has closed.
-pub fn job(rx: Arc<Mutex<Receiver<PathBuf>>>, cmd: Arc<CommandTemplate>, out_perm: Arc<Mutex<()>>) {
+pub fn job(
+    rx: Arc<Mutex<Receiver<WorkerResult>>>,
+    cmd: Arc<CommandTemplate>,
+    out_perm: Arc<Mutex<()>>,
+) {
     loop {
         // Create a lock on the shared receiver for this thread.
         let lock = rx.lock().unwrap();
 
-        // Obtain the next path from the receiver, else if the channel
+        // Obtain the next result from the receiver, else if the channel
         // has closed, exit from the loop
         let value: PathBuf = match lock.recv() {
-            Ok(value) => value,
+            Ok(WorkerResult::Entry(val)) => val,
+            Ok(WorkerResult::Error(err)) => {
+                print_error(&format!("{}", err));
+                continue;
+            }
             Err(_) => break,
         };
 
diff --git a/src/internal.rs b/src/internal.rs
index 263571d5a..78bf1ac06 100644
--- a/src/internal.rs
+++ b/src/internal.rs
@@ -7,7 +7,6 @@
 // according to those terms.
 
 use std::ffi::OsString;
-use std::io::Write;
 use std::path::PathBuf;
 use std::process;
 use std::time;
@@ -174,9 +173,14 @@ pub struct FdOptions {
     pub size_constraints: Vec<SizeFilter>,
 }
 
+/// Print error message to stderr.
+pub fn print_error(message: &str) {
+    eprintln!("{}", message);
+}
+
 /// Print error message to stderr and exit with status `1`.
-pub fn error(message: &str) -> ! {
-    writeln!(&mut ::std::io::stderr(), "{}", message).expect("Failed writing to stderr");
+pub fn print_error_and_exit(message: &str) -> ! {
+    print_error(message);
     process::exit(1);
 }
 
diff --git a/src/main.rs b/src/main.rs
index defcde580..69d86564f 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -38,7 +38,8 @@ use regex::{RegexBuilder, RegexSetBuilder};
 
 use exec::CommandTemplate;
 use internal::{
-    error, pattern_has_uppercase_char, transform_args_with_exec, FdOptions, FileTypes, SizeFilter,
+    pattern_has_uppercase_char, print_error_and_exit, transform_args_with_exec, FdOptions,
+    FileTypes, SizeFilter,
 };
 use lscolors::LsColors;
 
@@ -52,7 +53,7 @@ fn main() {
     // Get the current working directory
     let current_dir = Path::new(".");
     if !fshelper::is_dir(current_dir) {
-        error("Error: could not get current directory.");
+        print_error_and_exit("Error: could not get current directory.");
     }
 
     // Get one or more root directories to search.
@@ -61,7 +62,7 @@ fn main() {
             .map(|path| {
                 let path_buffer = PathBuf::from(path);
                 if !fshelper::is_dir(&path_buffer) {
-                    error(&format!(
+                    print_error_and_exit(&format!(
                         "Error: '{}' is not a directory.",
                         path_buffer.to_string_lossy()
                     ));
@@ -89,7 +90,7 @@ fn main() {
         && pattern.contains(std::path::MAIN_SEPARATOR)
         && fshelper::is_dir(Path::new(pattern))
     {
-        error(&format!(
+        print_error_and_exit(&format!(
             "Error: The search pattern '{pattern}' contains a path-separation character ('{sep}') \
              and will not lead to any search results.\n\n\
              If you want to search for all files inside the '{pattern}' directory, use a match-all pattern:\n\n  \
@@ -142,7 +143,7 @@ fn main() {
                 if let Some(f) = SizeFilter::from_string(sf) {
                     return f;
                 }
-                error(&format!("Error: {} is not a valid size constraint.", sf));
+                print_error_and_exit(&format!("Error: {} is not a valid size constraint.", sf));
             })
             .collect()
         })
@@ -211,7 +212,7 @@ fn main() {
                 .build()
             {
                 Ok(re) => re,
-                Err(err) => error(err.description()),
+                Err(err) => print_error_and_exit(err.description()),
             }
         }),
         command,
@@ -232,7 +233,7 @@ fn main() {
         .build()
     {
         Ok(re) => walk::scan(&dir_vec, Arc::new(re), Arc::new(config)),
-        Err(err) => error(
+        Err(err) => print_error_and_exit(
             format!(
                 "{}\nHint: You can use the '--fixed-strings' option to search for a \
                  literal string instead of a regular expression",
diff --git a/src/walk.rs b/src/walk.rs
index 689f76551..29672e7d5 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -10,7 +10,7 @@ extern crate ctrlc;
 
 use exec;
 use fshelper;
-use internal::{error, FdOptions, EXITCODE_SIGINT, MAX_BUFFER_LENGTH};
+use internal::{print_error, print_error_and_exit, FdOptions, EXITCODE_SIGINT, MAX_BUFFER_LENGTH};
 use output;
 
 use std::error::Error;
@@ -36,6 +36,12 @@ enum ReceiverMode {
     Streaming,
 }
 
+/// The Worker threads can result in a valid entry having PathBuf or an error.
+pub enum WorkerResult {
+    Entry(PathBuf),
+    Error(ignore::Error),
+}
+
 /// Recursively scan the given search path for files / pathnames matching the pattern.
 ///
 /// If the `--exec` argument was supplied, this will create a thread pool for executing
@@ -54,11 +60,11 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
     for pattern in &config.exclude_patterns {
         let res = override_builder.add(pattern);
         if res.is_err() {
-            error(&format!("Error: malformed exclude pattern '{}'", pattern));
+            print_error_and_exit(&format!("Error: malformed exclude pattern '{}'", pattern));
         }
     }
     let overrides = override_builder.build().unwrap_or_else(|_| {
-        error("Mismatch in exclude patterns");
+        print_error_and_exit("Mismatch in exclude patterns");
     });
 
     let mut walker = WalkBuilder::new(first_path_buf.as_path());
@@ -83,7 +89,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             match err {
                 ignore::Error::Partial(_) => (),
                 _ => {
-                    error(&format!(
+                    print_error_and_exit(&format!(
                         "Error while parsing custom ignore file '{}': {}.",
                         ignore_file.to_string_lossy(),
                         err.description()
@@ -156,27 +162,34 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
                 .max_buffer_time
                 .unwrap_or_else(|| time::Duration::from_millis(100));
 
-            for value in rx {
-                match mode {
-                    ReceiverMode::Buffering => {
-                        buffer.push(value);
-
-                        // Have we reached the maximum buffer size or maximum buffering time?
-                        if buffer.len() > MAX_BUFFER_LENGTH
-                            || time::Instant::now() - start > max_buffer_time
-                        {
-                            // Flush the buffer
-                            for v in &buffer {
-                                output::print_entry(v, &rx_config, &receiver_wtq);
+            for worker_result in rx {
+                match worker_result {
+                    WorkerResult::Entry(value) => {
+                        match mode {
+                            ReceiverMode::Buffering => {
+                                buffer.push(value);
+
+                                // Have we reached the maximum buffer size or maximum buffering time?
+                                if buffer.len() > MAX_BUFFER_LENGTH
+                                    || time::Instant::now() - start > max_buffer_time
+                                {
+                                    // Flush the buffer
+                                    for v in &buffer {
+                                        output::print_entry(v, &rx_config, &receiver_wtq);
+                                    }
+                                    buffer.clear();
+
+                                    // Start streaming
+                                    mode = ReceiverMode::Streaming;
+                                }
+                            }
+                            ReceiverMode::Streaming => {
+                                output::print_entry(&value, &rx_config, &receiver_wtq);
                             }
-                            buffer.clear();
-
-                            // Start streaming
-                            mode = ReceiverMode::Streaming;
                         }
                     }
-                    ReceiverMode::Streaming => {
-                        output::print_entry(&value, &rx_config, &receiver_wtq);
+                    WorkerResult::Error(err) => {
+                        print_error(&format!("{}", err));
                     }
                 }
             }
@@ -206,7 +219,10 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
 
             let entry = match entry_o {
                 Ok(e) => e,
-                Err(_) => return ignore::WalkState::Continue,
+                Err(err) => {
+                    tx_thread.send(WorkerResult::Error(err)).unwrap();
+                    return ignore::WalkState::Continue;
+                }
             };
 
             if entry.depth() == 0 {
@@ -275,7 +291,7 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             let search_str_o = if config.search_full_path {
                 match fshelper::path_absolute_form(entry_path) {
                     Ok(path_abs_buf) => Some(path_abs_buf.to_string_lossy().into_owned().into()),
-                    Err(_) => error("Error: unable to get full path."),
+                    Err(_) => print_error_and_exit("Error: unable to get full path."),
                 }
             } else {
                 entry_path.file_name().map(|f| f.to_string_lossy())
@@ -284,7 +300,9 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             if let Some(search_str) = search_str_o {
                 if pattern.is_match(&*search_str) {
                     // TODO: take care of the unwrap call
-                    tx_thread.send(entry_path.to_owned()).unwrap()
+                    tx_thread
+                        .send(WorkerResult::Entry(entry_path.to_owned()))
+                        .unwrap()
                 }
             }
 


Test patch: