Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should show indication of local files/directories that can't be scanned for whatever reason #4480

Closed
generalmanager opened this Issue Nov 3, 2017 · 11 comments

Comments

Projects
None yet
6 participants
@generalmanager
Copy link

commented Nov 3, 2017

First of all: sorry if I missed any similar tickets, surprisingly I couldn't find any.

While debugging a syncthing-setup I noticed that lots of files were counted but not transmitted because they were created by root and unreadable by the user.
They were very much listable by the user tough, so I was pretty amazed that we don't already notify the user of such and similar problematic cases such as symlinks.

I'm pretty sure the best way to signal this would be to change the folder status to "out of sync" with the same visual representation but a new status similar to "unsupported type or file/directory unreadable". I'd also like to change the color of the "downloading"status to something less alarming and reserve red as a clear warning color for this unfortunate state. In addition files/folders with such problems should be listed on top of the other out of sync files and the whole line should be permanently red, to underscore that the user has to do something about it.

Now there is the rare case of people who knowingly add such directories and still want to sync the other stuff apart from unreadable or symlinked files/folders. The ideal solution would be to keep a list of ignored paths per shared folder, which don't trigger this warning with a nice "ignore this path in the future" button next to the warning in the out of sync dialog.

But I'd be perfectly happy to have a general "don't show me such warnings for this shared folder" boolean for now, even though it doesn't cover the case of shared folders with known problematic paths, where other unsyncable files are unknowingly added later on.
In this case one will never be notified of the new problems, because these kinds of errors are completely ignored already.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Nov 3, 2017

Nor does one drive ir dropbox or anything else complain about unreadable files. Files can be unreadable for a temporary period, etc.

I vote we close this as "won't fix".

@generalmanager

This comment has been minimized.

Copy link
Author

commented Nov 3, 2017

Just because other solutions don't do any better and this might be more cumbersome than it seems at first look you aren't interested in even trying to notify the user of possible problems?

If the ephermality of some of those cases is your main problem, we could either build the advanced solution described above with a list of paths to ignore or simply make the warning itself temporary and show it inside our typical warning/notification boxes at the top of the webinterface.

We could also think about the warning only beeing triggered if there is more than one such file or a directory we can't enter (so we don't know how many files are affected). This way most cases like a single lockfile beeing open don't even show up on the users radar.

Most users will only have a very small number of files (if any) which could trigger such a warning, like lock files of backup or editing programs. If we did build the "big solution" with the ignore list, the majority of all users wouldn't have to do anything, while some might have to click ignore a handful of times.
Which is a rather low barrier and not very deterrent from a user experience view.

But if even one user loses important data because of this, they'll most likely blame syncthing for it and discourage others from using it. Which is not just bad for the reputation, but a sign that it failed to do something the user expected of it.

I've been using syncthing since the very beginning with dozens of devices and several TB of synced files (not mine, managing a friends&family cluster). So I do know all the common fallacies, but I still spent more time on this than I'd like to admit until I noticed that hundreds of files in one of the subsubdirectories were missing on one machine. The easiest way on linux to find out are rsync or using diff on an sshfs-mounted directory. I'd assume that windows users don't have it that easy.

I do realize there are more difficult cases like a file beeing written to or held open by another program, but most of those cases shouldn't be too difficult to identify and ignore. Not doing something the user expects and which touches the core functionality seems like something we should try to improve. Especially because we are throwing warnings and notifications for so many other things.

@calmh

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

More generally I don't think we expose scanner errors anywhere useful at all at the moment?

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

Well, I argue we shouldn't, as there might be transient non-critical errors, or permission errors.

@calmh

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

The desired loudness level is certainly debatable, but a list of things that could not be accessed, somewhere in the GUI, makes sense to me.

@imsodin

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

I understand transient non-critical errors, but how are potentially permanent permission errors non-critical? If I set up a dir to sync, forgetting that the user Syncthing runs under has not sufficient permissions, I currently don't get any indication that something is wrong unless a peer sends an index update for those files. There is an asymmetry between pulling, where we report errors, regardless whether they are transient or not, and scanning, where we don't report anything at all.

@generalmanager

This comment has been minimized.

Copy link
Author

commented Nov 9, 2017

We could cache such notifications for a few hours to weed out transient stuff. But I don't really understand why you seem so fundamentally opposed to notifying of serious/permanent scanner errors @AudriusButkevicius when we are already exposing more or less any pulling errors, short time or not as @imsodin mentioned.

Sure too many popups and warnings clutter the UI and may confuse the user. But if that's your concern, we could filter some of the transient puller errors and make sure to only notify of the more serious scanner errors.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

If we do something like puller failed items where there are no popups just small indication, it's probably ok. Otherwise it would be annoying. But we'd have to not bulldoze pull errors etc.

@calmh calmh changed the title [UX] "Out of sync" status for unreadable/unsupported filetypes Should show indication of local files/directories that can't be scanned for whatever reason Nov 17, 2017

@calmh calmh added the enhancement label Nov 17, 2017

@calmh calmh added this to the Unplanned (Contributions Welcome) milestone Nov 17, 2017

@calmh calmh removed this from the Unplanned (Contributions Welcome) milestone Feb 11, 2018

@NiceGuyIT

This comment has been minimized.

Copy link

commented May 31, 2018

+1 for notifications of files that could not be read.

Use case: Why do .zip files from Mac OS show up as green/encrypted?

When a Windows user unzips a file created by a Mac user, the encryption flag is inadvertently set preventing other users (like Syncthing) from reading it. Syncthing is run as a service on the file server and silently ignoring files that can't be read means Syncthing failed to do it's primary job: sync files between two servers. If Syncthing doesn't notify about failed files due to permission issues, then extra effort is required to identify failed files, whether that's shipping the logs to Elasticsearch, comparing the directories on two servers, or some other means.

Last I checked, that bug is fixed in the latest version of Windows 10.

imsodin added a commit to imsodin/syncthing that referenced this issue Sep 21, 2018

@bjornfor

This comment has been minimized.

Copy link

commented Oct 8, 2018

Simple improvement suggestion: Please increase the severity of the debug(!) log message that says syncthing cannot open files due to permissinon errors.

(I just spent some time debugging why syncthing didn't sync and eventually learned about the STTRACE=scanner environment variable. Without that, I was blind.)

@calmh calmh closed this in d510e3c Nov 7, 2018

@bjornfor

This comment has been minimized.

Copy link

commented Nov 7, 2018

Thanks!

@calmh calmh added this to the v0.14.53 milestone Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.