[Feature request] Improved handling for error messages

[valoq]

There are several cases of confusing (error) messages when copying or moving files in lf:

Create a folder "test" with folders a, b and c inside.
The folder test has owner and permissions for the user running lf
The subfolders do not have the correct permissions, except for folder c

1. navigate to folder "test" and move it somewhere else
2. lf will display error messages about the missing permissions for a split second, but the last message will show "Moved successfully" in green
3. in cases of large folder collections with some permissions missing, I only saw the green "Moved successfully" message, but no file was actually moved

When the same is done using copy instead of move, the files that have the right permissions are actually copied and the error message is shown in red, indicating the last failed copy operation.

There are two issues here though probably connected:

1. Move shows successful message even though no file was moved
2. Multiple error messages overwrite each other and cause confusion about what failed and what succeeded

Not sure how to best address this, but I would suggest that:

A) if there is an error, no success message may overwrite it
B) if there are multiple error messages, it should not show the last but a separate message about multiple errors

[joelim-work]

I can't reproduce, but the number of errors is tracked by the errCount variable and the message Moved successfully message only displays if it is 0. You can try debugging the moveAsync function though, it is possible there is some error code path that doesn't increment errCount.

[valoq]

I can't reproduce

I assume you mean the success message, since there is always only one error message displayed even in cases of multiple errors.

As for the issue of sucess message overwriting error messages try this poc:

#!/bin/bash
mkdir /tmp/testing-lf-bug
mkdir /tmp/testing-lf-bug/a
mkdir /tmp/testing-lf-bug/b
mkdir /tmp/testing-lf-bug/c
chmod 000 /tmp/testing-lf-bug/b
cd /tmp
lf
# use lf to move testing-lf-bug to $HOME

[joelim-work]

OK so the reason I wasn't able to reproduce it before was because I didn't create the directory in /tmp which usually a separate tmpfs filesystem, so an actual move is not possible and a copy has to be performed. That ends up hitting a different code path, and there was a missing errCount++. This means errCount ended up being 0 and lf thought there were no errors. Can you try #2085 to see if you no longer get the Moved successfully error message?

As for multiple issues, I don't think there's a good solution - the UI can only display one line of error message, and actually the total error count is displayed along with the last error. Hopefully that is good enough for most users though.

[valoq]

Yes, that fixed the issue and now the error message is displayed, thank you.

As for multiple error message, I would argue that a generic "multiple errors" message would be better, ideally with the option to open a log file that lists all errors

[joelim-work]

I thought about the idea of logging all the errors too (with a simple log.Printf("...") call), the only problem is that by default lf doesn't write to a log file unless the user specifies the -log command line flag when running it. Do you normally enable the log file in lf for day to day use? If so I can think about changing the code to log every error instead, but I still want to at least maintain the current behavior of displaying the last error for users that don't enable the log file.

[joelim-work]

So actually the intention of the code is to write each error to the log file, but there was a bug where if there were 10 errors, instead of all 10 of them being written, the last error would be written 10 times. This is fixed by #2089.

[valoq]

I thought about the idea of logging all the errors too (with a simple log.Printf("...") call), the only problem is that by default lf doesn't write to a log file unless the user specifies the -log command line flag when running it. Do you normally enable the log file in lf for day to day use? If so I can think about changing the code to log every error instead, but I still want to at least maintain the current behavior of displaying the last error for users that don't enable the log file.

The general idea was to "save" the error message in a temporary variable and after displaying the message about "multiple errors" the user could be offered to use command to show the temporary log, which does not live in the filesystem but only in the current session.
That would be a feature on its own though I guess

[joelim-work]

Yes, that technically is a feature request. I'm not really sure if it makes sense to implement it though, since the idea of having a "temporary log" indicates that the original -log functionality is lacking in some way.

Do you have an issue with creating log files on your system? Since you want to log all issues that occur anyway, you could use mktemp to create a temporary log file, and a custom command to view it:

lf -log "$(mktemp --tmpdir lf_XXX.log)"

lfrc:

cmd log ${{
    less "$lf_flag_log"
}}

[valoq]

Do you have an issue with creating log files on your system?

Its not that I don't want a logfile in the filesystem, but that I consider this kind of "error" message to be an entirely different kind then "debug" messages generated with the -log option. In my understanding, error messages printed to the file specified with -log are developer information indicating things that happen internally in lf.
The log I had in mind is for displaying errors from "external" factors such as trying to access a file with missing permissions. That's a log with user information that I would separate from the developer information related to lf.

If you don't oppose the idea of such a "log buffer" that can be displayed on demand when multiple (filesystem) errors occur, I may look into that and submit a PR soon.

[joelim-work]

So there is some earlier discussion on this in #1377

At this point, I can only think of a single disadvantage.

• Log files serve as a message history for cases when a message overwrites a previous message. An example use case for this is errors during file copying. We currently display the number of total errors beside the last error message if there are multiple errors. The idea is that a user can look up other errors from the log if desired.

I don't know whether or not this last point is useful in practice. Personally, I don't remember looking up previous copy errors from the log myself. Also, we might be able to add this without using a log file later on if desired. For example, I was thinking we could store log messages in a string and export them in an environment variable (i.e. $lf_log) so that users can use a shell command to display the log (i.e. echo $lf_log | less). I'm sure there are better alternatives.

I don't think it's a good idea to export the log as an environment variable, it just contributes to the problem in #1873 where the size of exported information is limited by ARG_MAX. If you want to separate user and developer information, the proper way to do that is to use log levels, for example using slog. This is probably a big task though, especially having to go through all the existing log messages and categorize them.

You can also just use grep to filter out only the entries you care about in the log file.

[valoq]

So there is some earlier discussion on this in #1377

There seems to be a typo since the linked issue does not seem related :)

We currently display the number of total errors beside the last error message

I don't see the total number though, even when multiple errors occur. Is this another bug or was it removed?

I don't think it's a good idea to export the log as an environment variable, it just contributes to the problem in #1873 where the size of exported information is limited by ARG_MAX. If you want to separate user and developer information, the proper way to do that is to use log levels, for example using slog. This is probably a big task though, especially having to go through all the existing log messages and categorize them.

The idea was to simply offer to expand the error message on demand to display all errors on the current operation within the gui itself, similar to how vim does it e.g. for they help, like when using :h

So instead of having to run lf with the debug command and then having to find and open the log file to filter out specific messages, the user could press a single button to show the full error.

[joelim-work]

Oops sorry, I linked the wrong issue, it should be #739.

Wouldn't implementing the display natively inside lf introduce more complexity into the code? It should be simple enough to append errors to a string and then set it to app.ui.menu, or else just directly draw it in the UI code, but then what if there are hundreds of error messages and the user wants to scroll through them? Or if the user wants to search using /?

Also how do you define what error messages are stored, is it just just errors from cut/copy, or are other error messages like unknown mapping: also included? What if some other user wants to include a different set of errors as well?

[valoq]

Perhaps that is a bit too much yes. Especially since we do have this information in the log already now.

A more simple approach could be to write the lf log file by default to $XDG_CACHE_HOME/lf/lflog and when multiple error occurs offer to open this log file directly by a shortcut in lf.
The log file usually does not have much included unless the debug mode is used, so there should not be any drawback from this, right?

Update: I should have read the newly linked issue first :)

Update2: After reading through the past issue, I think this could be added to lfrc along with the shortcut to open the logfile. That said, I still believe the case of multiple issues is quite relevant and important to all user.
If I move a directory with important files and there are errors during the operation, I want to know exactly what errors happened to which files. It wont help to lean afterwards that I could have used some option that is not the default. The only way to address this is to record all file operation errors by default and offer a direct and immediate way to read those logs.

As of now I don't even see if there are multiple error, lf will only show me a permission denied error on file x and hide the fact that files a, b and c had the same error and were not moved or copied in the operation. I may manually check on file x and never know that the others filed to be copied until I try to use them and its too late to fix this.
Update3: Though this is not mitigated by aborting the entire operation as I just noticed, so its not that bad I guess.

[joelim-work]

I searched through the code - some log.Printf calls are used to record internal events (e.g. a connecting to the server, loading a directory, etc.), but there are also a large number of log.Printf calls which record failed system calls or invalid user input (i.e. what you describe as "external"). So I would say that the log file is more of a general purpose log than just developer information used for submitting bug reports.

Of course, you are right that most users probably don't enable logs (or even know about it at all), so by default there's no way to view every single error in the case that there are multiple error messages. I'm not sure how much it matters in practice - usually just seeing any error message during copying is enough for the user to know it wasn't done reliably and that they need to investigate further. So I guess that's why the -log option was disabled by default to keep the default behavior simple, and advanced users can configure it for their needs.