all: Display errors while scanning in web UI (fixes #4480) (#5215)

cmd/syncthing/gui.go

@@ -115,7 +115,7 @@ type modelIntf interface {
 	RemoteSequence(folder string) (int64, bool)
 	State(folder string) (string, time.Time, error)
 	UsageReportingStats(version int, preview bool) map[string]interface{}
-	PullErrors(folder string) ([]model.FileError, error)
+	FolderErrors(folder string) ([]model.FileError, error)
 	WatchError(folder string) error
 }
 
@@ -261,7 +261,8 @@ func (s *apiService) Serve() {
 	getRestMux.HandleFunc("/rest/db/status", s.getDBStatus)                      // folder
 	getRestMux.HandleFunc("/rest/db/browse", s.getDBBrowse)                      // folder [prefix] [dirsonly] [levels]
 	getRestMux.HandleFunc("/rest/folder/versions", s.getFolderVersions)          // folder
-	getRestMux.HandleFunc("/rest/folder/pullerrors", s.getPullErrors)            // folder
+	getRestMux.HandleFunc("/rest/folder/errors", s.getFolderErrors)              // folder
+	getRestMux.HandleFunc("/rest/folder/pullerrors", s.getFolderErrors)          // folder (deprecated)
 	getRestMux.HandleFunc("/rest/events", s.getIndexEvents)                      // [since] [limit] [timeout] [events]
 	getRestMux.HandleFunc("/rest/events/disk", s.getDiskEvents)                  // [since] [limit] [timeout]
 	getRestMux.HandleFunc("/rest/stats/device", s.getDeviceStats)                // -
@@ -695,12 +696,13 @@ func (s *apiService) getDBStatus(w http.ResponseWriter, r *http.Request) {
 func folderSummary(cfg configIntf, m modelIntf, folder string) (map[string]interface{}, error) {
 	var res = make(map[string]interface{})
 
-	pullErrors, err := m.PullErrors(folder)
+	errors, err := m.FolderErrors(folder)
 	if err != nil && err != model.ErrFolderPaused {
 		// Stats from the db can still be obtained if the folder is just paused
 		return nil, err
 	}
-	res["pullErrors"] = len(pullErrors)
+	res["errors"] = len(errors)
+	res["pullErrors"] = len(errors) // deprecated
 
 	res["invalid"] = "" // Deprecated, retains external API for now
 
@@ -1501,12 +1503,12 @@ func (s *apiService) postFolderVersionsRestore(w http.ResponseWriter, r *http.Re
 	sendJSON(w, ferr)
 }
 
-func (s *apiService) getPullErrors(w http.ResponseWriter, r *http.Request) {
+func (s *apiService) getFolderErrors(w http.ResponseWriter, r *http.Request) {
 	qs := r.URL.Query()
 	folder := qs.Get("folder")
 	page, perpage := getPagingParams(qs)
 
-	errors, err := s.model.PullErrors(folder)
+	errors, err := s.model.FolderErrors(folder)
 
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusNotFound)

cmd/syncthing/mocked_model_test.go

@@ -139,7 +139,7 @@ func (m *mockedModel) UsageReportingStats(version int, preview bool) map[string]
 	return nil
 }
 
-func (m *mockedModel) PullErrors(folder string) ([]model.FileError, error) {
+func (m *mockedModel) FolderErrors(folder string) ([]model.FileError, error) {
 	return nil, nil
 }
 

lib/model/folder.go

@@ -11,14 +11,20 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
+	"path/filepath"
+	"sort"
+	"strings"
 	"sync/atomic"
 	"time"
 
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/ignore"
+	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
+	"github.com/syncthing/syncthing/lib/scanner"
 	"github.com/syncthing/syncthing/lib/sync"
 	"github.com/syncthing/syncthing/lib/watchaggregator"
 )
@@ -41,6 +47,8 @@ type folder struct {
 	scanDelay           chan time.Duration
 	initialScanFinished chan struct{}
 	stopped             chan struct{}
+	scanErrors          []FileError
+	scanErrorsMut       sync.Mutex
 
 	pullScheduled chan struct{}
 
@@ -80,6 +88,7 @@ func newFolder(model *Model, cfg config.FolderConfiguration) folder {
 		scanDelay:           make(chan time.Duration),
 		initialScanFinished: make(chan struct{}),
 		stopped:             make(chan struct{}),
+		scanErrorsMut:       sync.NewMutex(),
 
 		pullScheduled: make(chan struct{}, 1), // This needs to be 1-buffered so that we queue a pull if we're busy when it comes.
 
@@ -266,14 +275,255 @@ func (f *folder) getHealthError() error {
 }
 
 func (f *folder) scanSubdirs(subDirs []string) error {
-	if err := f.model.internalScanFolderSubdirs(f.ctx, f.folderID, subDirs, f.localFlags); err != nil {
+	if err := f.CheckHealth(); err != nil {
-		// Potentially sets the error twice, once in the scanner just
+		return err
-		// by doing a check, and once here, if the error returned is
+	}
-		// the same one as returned by CheckHealth, though
+
-		// duplicate set is handled by setError.
+	f.model.fmut.RLock()
+	fset := f.model.folderFiles[f.ID]
+	ignores := f.model.folderIgnores[f.ID]
+	f.model.fmut.RUnlock()
+	mtimefs := fset.MtimeFS()
+
+	for i := range subDirs {
+		sub := osutil.NativeFilename(subDirs[i])
+
+		if sub == "" {
+			// A blank subdirs means to scan the entire folder. We can trim
+			// the subDirs list and go on our way.
+			subDirs = nil
+			break
+		}
+
+		subDirs[i] = sub
+	}
+
+	// Check if the ignore patterns changed as part of scanning this folder.
+	// If they did we should schedule a pull of the folder so that we
+	// request things we might have suddenly become unignored and so on.
+	oldHash := ignores.Hash()
+	defer func() {
+		if ignores.Hash() != oldHash {
+			l.Debugln("Folder", f.ID, "ignore patterns changed; triggering puller")
+			f.IgnoresUpdated()
+		}
+	}()
+
+	if err := ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) {
+		err = fmt.Errorf("loading ignores: %v", err)
 		f.setError(err)
 		return err
 	}
+
+	// Clean the list of subitems to ensure that we start at a known
+	// directory, and don't scan subdirectories of things we've already
+	// scanned.
+	subDirs = unifySubs(subDirs, func(f string) bool {
+		_, ok := fset.Get(protocol.LocalDeviceID, f)
+		return ok
+	})
+
+	f.setState(FolderScanning)
+
+	fchan := scanner.Walk(f.ctx, scanner.Config{
+		Folder:                f.ID,
+		Subs:                  subDirs,
+		Matcher:               ignores,
+		TempLifetime:          time.Duration(f.model.cfg.Options().KeepTemporariesH) * time.Hour,
+		CurrentFiler:          cFiler{f.model, f.ID},
+		Filesystem:            mtimefs,
+		IgnorePerms:           f.IgnorePerms,
+		AutoNormalize:         f.AutoNormalize,
+		Hashers:               f.model.numHashers(f.ID),
+		ShortID:               f.model.shortID,
+		ProgressTickIntervalS: f.ScanProgressIntervalS,
+		UseLargeBlocks:        f.UseLargeBlocks,
+		LocalFlags:            f.localFlags,
+	})
+
+	batchFn := func(fs []protocol.FileInfo) error {
+		if err := f.CheckHealth(); err != nil {
+			l.Debugf("Stopping scan of folder %s due to: %s", f.Description(), err)
+			return err
+		}
+		f.model.updateLocalsFromScanning(f.ID, fs)
+		return nil
+	}
+	// Resolve items which are identical with the global state.
+	if f.localFlags&protocol.FlagLocalReceiveOnly != 0 {
+		oldBatchFn := batchFn // can't reference batchFn directly (recursion)
+		batchFn = func(fs []protocol.FileInfo) error {
+			for i := range fs {
+				switch gf, ok := fset.GetGlobal(fs[i].Name); {
+				case !ok:
+					continue
+				case gf.IsEquivalentOptional(fs[i], false, false, protocol.FlagLocalReceiveOnly):
+					// What we have locally is equivalent to the global file.
+					fs[i].Version = fs[i].Version.Merge(gf.Version)
+					fallthrough
+				case fs[i].IsDeleted() && gf.IsReceiveOnlyChanged():
+					// Our item is deleted and the global item is our own
+					// receive only file. We can't delete file infos, so
+					// we just pretend it is a normal deleted file (nobody
+					// cares about that).
+					fs[i].LocalFlags &^= protocol.FlagLocalReceiveOnly
+				}
+			}
+			return oldBatchFn(fs)
+		}
+	}
+	batch := newFileInfoBatch(batchFn)
+
+	// Schedule a pull after scanning, but only if we actually detected any
+	// changes.
+	changes := 0
+	defer func() {
+		if changes > 0 {
+			f.SchedulePull()
+		}
+	}()
+
+	f.clearScanErrors(subDirs)
+	for res := range fchan {
+		if res.Err != nil {
+			f.newScanError(res.Path, res.Err)
+			continue
+		}
+		if err := batch.flushIfFull(); err != nil {
+			return err
+		}
+
+		batch.append(res.File)
+		changes++
+	}
+
+	if err := batch.flush(); err != nil {
+		return err
+	}
+
+	if len(subDirs) == 0 {
+		// If we have no specific subdirectories to traverse, set it to one
+		// empty prefix so we traverse the entire folder contents once.
+		subDirs = []string{""}
+	}
+
+	// Do a scan of the database for each prefix, to check for deleted and
+	// ignored files.
+	var toIgnore []db.FileInfoTruncated
+	ignoredParent := ""
+	pathSep := string(fs.PathSeparator)
+	for _, sub := range subDirs {
+		var iterError error
+
+		fset.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool {
+			file := fi.(db.FileInfoTruncated)
+
+			if err := batch.flushIfFull(); err != nil {
+				iterError = err
+				return false
+			}
+
+			if ignoredParent != "" && !strings.HasPrefix(file.Name, ignoredParent+pathSep) {
+				for _, file := range toIgnore {
+					l.Debugln("marking file as ignored", file)
+					nf := file.ConvertToIgnoredFileInfo(f.model.id.Short())
+					batch.append(nf)
+					changes++
+					if err := batch.flushIfFull(); err != nil {
+						iterError = err
+						return false
+					}
+				}
+				toIgnore = toIgnore[:0]
+				ignoredParent = ""
+			}
+
+			switch ignored := ignores.Match(file.Name).IsIgnored(); {
+			case !file.IsIgnored() && ignored:
+				// File was not ignored at last pass but has been ignored.
+				if file.IsDirectory() {
+					// Delay ignoring as a child might be unignored.
+					toIgnore = append(toIgnore, file)
+					if ignoredParent == "" {
+						// If the parent wasn't ignored already, set
+						// this path as the "highest" ignored parent
+						ignoredParent = file.Name
+					}
+					return true
+				}
+
+				l.Debugln("marking file as ignored", f)
+				nf := file.ConvertToIgnoredFileInfo(f.model.id.Short())
+				batch.append(nf)
+				changes++
+
+			case file.IsIgnored() && !ignored:
+				// Successfully scanned items are already un-ignored during
+				// the scan, so check whether it is deleted.
+				fallthrough
+			case !file.IsIgnored() && !file.IsDeleted() && !file.IsUnsupported():
+				// The file is not ignored, deleted or unsupported. Lets check if
+				// it's still here. Simply stat:ing it wont do as there are
+				// tons of corner cases (e.g. parent dir->symlink, missing
+				// permissions)
+				if !osutil.IsDeleted(mtimefs, file.Name) {
+					if ignoredParent != "" {
+						// Don't ignore parents of this not ignored item
+						toIgnore = toIgnore[:0]
+						ignoredParent = ""
+					}
+					return true
+				}
+				nf := protocol.FileInfo{
+					Name:       file.Name,
+					Type:       file.Type,
+					Size:       0,
+					ModifiedS:  file.ModifiedS,
+					ModifiedNs: file.ModifiedNs,
+					ModifiedBy: f.model.id.Short(),
+					Deleted:    true,
+					Version:    file.Version.Update(f.model.shortID),
+					LocalFlags: f.localFlags,
+				}
+				// We do not want to override the global version
+				// with the deleted file. Keeping only our local
+				// counter makes sure we are in conflict with any
+				// other existing versions, which will be resolved
+				// by the normal pulling mechanisms.
+				if file.ShouldConflict() {
+					nf.Version = nf.Version.DropOthers(f.model.shortID)
+				}
+
+				batch.append(nf)
+				changes++
+			}
+			return true
+		})
+
+		if iterError == nil && len(toIgnore) > 0 {
+			for _, file := range toIgnore {
+				l.Debugln("marking file as ignored", f)
+				nf := file.ConvertToIgnoredFileInfo(f.model.id.Short())
+				batch.append(nf)
+				changes++
+				if iterError = batch.flushIfFull(); iterError != nil {
+					break
+				}
+			}
+			toIgnore = toIgnore[:0]
+		}
+
+		if iterError != nil {
+			return iterError
+		}
+	}
+
+	if err := batch.flush(); err != nil {
+		return err
+	}
+
+	f.model.folderStatRef(f.ID).ScanCompleted()
+	f.setState(FolderIdle)
 	return nil
 }
 
@@ -430,3 +680,73 @@ func (f *folder) basePause() time.Duration {
 func (f *folder) String() string {
 	return fmt.Sprintf("%s/%s@%p", f.Type, f.folderID, f)
 }
+
+func (f *folder) newScanError(path string, err error) {
+	f.scanErrorsMut.Lock()
+	f.scanErrors = append(f.scanErrors, FileError{
+		Err:  err.Error(),
+		Path: path,
+	})
+	f.scanErrorsMut.Unlock()
+}
+
+func (f *folder) clearScanErrors(subDirs []string) {
+	f.scanErrorsMut.Lock()
+	defer f.scanErrorsMut.Unlock()
+	if len(subDirs) == 0 {
+		f.scanErrors = nil
+		return
+	}
+	filtered := f.scanErrors[:0]
+	pathSep := string(fs.PathSeparator)
+outer:
+	for _, fe := range f.scanErrors {
+		for _, sub := range subDirs {
+			if strings.HasPrefix(fe.Path, sub+pathSep) {
+				continue outer
+			}
+		}
+		filtered = append(filtered, fe)
+	}
+	f.scanErrors = filtered
+}
+
+func (f *folder) Errors() []FileError {
+	f.scanErrorsMut.Lock()
+	defer f.scanErrorsMut.Unlock()
+	return append([]FileError{}, f.scanErrors...)
+}
+
+// The exists function is expected to return true for all known paths
+// (excluding "" and ".")
+func unifySubs(dirs []string, exists func(dir string) bool) []string {
+	if len(dirs) == 0 {
+		return nil
+	}
+	sort.Strings(dirs)
+	if dirs[0] == "" || dirs[0] == "." || dirs[0] == string(fs.PathSeparator) {
+		return nil
+	}
+	prev := "./" // Anything that can't be parent of a clean path
+	for i := 0; i < len(dirs); {
+		dir, err := fs.Canonicalize(dirs[i])
+		if err != nil {
+			l.Debugf("Skipping %v for scan: %s", dirs[i], err)
+			dirs = append(dirs[:i], dirs[i+1:]...)
+			continue
+		}
+		if dir == prev || strings.HasPrefix(dir, prev+string(fs.PathSeparator)) {
+			dirs = append(dirs[:i], dirs[i+1:]...)
+			continue
+		}
+		parent := filepath.Dir(dir)
+		for parent != "." && parent != string(fs.PathSeparator) && !exists(parent) {
+			dir = parent
+			parent = filepath.Dir(dir)
+		}
+		dirs[i] = dir
+		prev = dir
+		i++
+	}
+	return dirs
+}

lib/model/folder_sendrecv.go

@@ -102,8 +102,8 @@ type sendReceiveFolder struct {
 
 	queue *jobQueue
 
-	errors    map[string]string // path -> error string
+	pullErrors    map[string]string // path -> error string
-	errorsMut sync.Mutex
+	pullErrorsMut sync.Mutex
 }
 
 func newSendReceiveFolder(model *Model, cfg config.FolderConfiguration, ver versioner.Versioner, fs fs.Filesystem) service {
@@ -112,7 +112,7 @@ func newSendReceiveFolder(model *Model, cfg config.FolderConfiguration, ver vers
 		fs:            fs,
 		versioner:     ver,
 		queue:         newJobQueue(),
-		errorsMut: sync.NewMutex(),
+		pullErrorsMut: sync.NewMutex(),
 	}
 	f.folder.puller = f
 
@@ -167,7 +167,7 @@ func (f *sendReceiveFolder) pull() bool {
 	l.Debugf("%v pulling (ignoresChanged=%v)", f, ignoresChanged)
 
 	f.setState(FolderSyncing)
-	f.clearErrors()
+	f.clearPullErrors()
 
 	scanChan := make(chan string)
 	go f.pullScannerRoutine(scanChan)
@@ -204,10 +204,10 @@ func (f *sendReceiveFolder) pull() bool {
 			// we're not making it. Probably there are write
 			// errors preventing us. Flag this with a warning and
 			// wait a bit longer before retrying.
-			if folderErrors := f.PullErrors(); len(folderErrors) > 0 {
+			if errors := f.Errors(); len(errors) > 0 {
 				events.Default.Log(events.FolderErrors, map[string]interface{}{
 					"folder": f.folderID,
-					"errors": folderErrors,
+					"errors": errors,
 				})
 			}
 			break
@@ -327,7 +327,7 @@ func (f *sendReceiveFolder) processNeeded(ignores *ignore.Matcher, folderFiles *
 			changed++
 
 		case runtime.GOOS == "windows" && fs.WindowsInvalidFilename(file.Name):
-			f.newError("pull", file.Name, fs.ErrInvalidFilename)
+			f.newPullError("pull", file.Name, fs.ErrInvalidFilename)
 
 		case file.IsDeleted():
 			if file.IsDirectory() {
@@ -497,7 +497,7 @@ nextFile:
 				continue nextFile
 			}
 		}
-		f.newError("pull", fileName, errNotAvailable)
+		f.newPullError("pull", fileName, errNotAvailable)
 	}
 
 	return changed, fileDeletions, dirDeletions, nil
@@ -513,7 +513,7 @@ func (f *sendReceiveFolder) processDeletions(ignores *ignore.Matcher, fileDeleti
 
 		l.Debugln(f, "Deleting file", file.Name)
 		if update, err := f.deleteFile(file, scanChan); err != nil {
-			f.newError("delete file", file.Name, err)
+			f.newPullError("delete file", file.Name, err)
 		} else {
 			dbUpdateChan <- update
 		}
@@ -573,7 +573,7 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan<
 	case err == nil && (!info.IsDir() || info.IsSymlink()):
 		err = osutil.InWritableDir(f.fs.Remove, f.fs, file.Name)
 		if err != nil {
-			f.newError("dir replace", file.Name, err)
+			f.newPullError("dir replace", file.Name, err)
 			return
 		}
 		fallthrough
@@ -603,13 +603,13 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan<
 		if err = osutil.InWritableDir(mkdir, f.fs, file.Name); err == nil {
 			dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleDir}
 		} else {
-			f.newError("dir mkdir", file.Name, err)
+			f.newPullError("dir mkdir", file.Name, err)
 		}
 		return
 	// Weird error when stat()'ing the dir. Probably won't work to do
 	// anything else with it if we can't even stat() it.
 	case err != nil:
-		f.newError("dir stat", file.Name, err)
+		f.newPullError("dir stat", file.Name, err)
 		return
 	}
 
@@ -621,7 +621,7 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan<
 	} else if err := f.fs.Chmod(file.Name, mode|(fs.FileMode(info.Mode())&retainBits)); err == nil {
 		dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleDir}
 	} else {
-		f.newError("dir chmod", file.Name, err)
+		f.newPullError("dir chmod", file.Name, err)
 	}
 }
 
@@ -631,7 +631,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo
 	parent := filepath.Dir(file)
 
 	if err := osutil.TraversesSymlink(f.fs, parent); err != nil {
-		f.newError("traverses q", file, err)
+		f.newPullError("traverses q", file, err)
 		return false
 	}
 
@@ -652,7 +652,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo
 	}
 	l.Debugf("%v resurrecting parent directory of %v", f, file)
 	if err := f.fs.MkdirAll(parent, 0755); err != nil {
-		f.newError("resurrecting parent dir", file, err)
+		f.newPullError("resurrecting parent dir", file, err)
 		return false
 	}
 	scanChan <- parent
@@ -691,7 +691,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c
 		// Index entry from a Syncthing predating the support for including
 		// the link target in the index entry. We log this as an error.
 		err = errors.New("incompatible symlink entry; rescan with newer Syncthing on source")
-		f.newError("symlink", file.Name, err)
+		f.newPullError("symlink", file.Name, err)
 		return
 	}
 
@@ -701,7 +701,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c
 		// path.
 		err = osutil.InWritableDir(f.fs.Remove, f.fs, file.Name)
 		if err != nil {
-			f.newError("symlink remove", file.Name, err)
+			f.newPullError("symlink remove", file.Name, err)
 			return
 		}
 	}
@@ -715,7 +715,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c
 	if err = osutil.InWritableDir(createLink, f.fs, file.Name); err == nil {
 		dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleSymlink}
 	} else {
-		f.newError("symlink create", file.Name, err)
+		f.newPullError("symlink create", file.Name, err)
 	}
 }
 
@@ -743,7 +743,7 @@ func (f *sendReceiveFolder) handleDeleteDir(file protocol.FileInfo, ignores *ign
 	}()
 
 	if err = f.deleteDir(file.Name, ignores, scanChan); err != nil {
-		f.newError("delete dir", file.Name, err)
+		f.newPullError("delete dir", file.Name, err)
 		return
 	}
 
@@ -1018,7 +1018,7 @@ func (f *sendReceiveFolder) handleFile(file protocol.FileInfo, copyChan chan<- c
 	}
 
 	if err := f.CheckAvailableSpace(blocksSize); err != nil {
-		f.newError("pulling file", file.Name, err)
+		f.newPullError("pulling file", file.Name, err)
 		f.queue.Done(file.Name)
 		return
 	}
@@ -1130,7 +1130,7 @@ func (f *sendReceiveFolder) shortcutFile(file, curFile protocol.FileInfo, dbUpda
 
 	if !f.IgnorePerms && !file.NoPermissions {
 		if err = f.fs.Chmod(file.Name, fs.FileMode(file.Permissions&0777)); err != nil {
-			f.newError("shortcut", file.Name, err)
+			f.newPullError("shortcut", file.Name, err)
 			return
 		}
 	}
@@ -1543,7 +1543,7 @@ func (f *sendReceiveFolder) finisherRoutine(ignores *ignore.Matcher, in <-chan *
 			}
 
 			if err != nil {
-				f.newError("finisher", state.file.Name, err)
+				f.newPullError("finisher", state.file.Name, err)
 			} else {
 				blockStatsMut.Lock()
 				blockStats["total"] += state.reused + state.copyTotal + state.pullTotal
@@ -1768,34 +1768,36 @@ func (f *sendReceiveFolder) moveForConflict(name string, lastModBy string) error
 	return err
 }
 
-func (f *sendReceiveFolder) newError(context, path string, err error) {
+func (f *sendReceiveFolder) newPullError(context, path string, err error) {
-	f.errorsMut.Lock()
+	f.pullErrorsMut.Lock()
-	defer f.errorsMut.Unlock()
+	defer f.pullErrorsMut.Unlock()
 
 	// We might get more than one error report for a file (i.e. error on
 	// Write() followed by Close()); we keep the first error as that is
 	// probably closer to the root cause.
-	if _, ok := f.errors[path]; ok {
+	if _, ok := f.pullErrors[path]; ok {
 		return
 	}
 	l.Infof("Puller (folder %s, file %q): %s: %v", f.Description(), path, context, err)
-	f.errors[path] = fmt.Sprintf("%s: %s", context, err.Error())
+	f.pullErrors[path] = fmt.Sprintf("%s: %s", context, err.Error())
 }
 
-func (f *sendReceiveFolder) clearErrors() {
+func (f *sendReceiveFolder) clearPullErrors() {
-	f.errorsMut.Lock()
+	f.pullErrorsMut.Lock()
-	f.errors = make(map[string]string)
+	f.pullErrors = make(map[string]string)
-	f.errorsMut.Unlock()
+	f.pullErrorsMut.Unlock()
 }
 
-func (f *sendReceiveFolder) PullErrors() []FileError {
+func (f *sendReceiveFolder) Errors() []FileError {
-	f.errorsMut.Lock()
+	scanErrors := f.folder.Errors()
-	errors := make([]FileError, 0, len(f.errors))
+	f.pullErrorsMut.Lock()
-	for path, err := range f.errors {
+	errors := make([]FileError, 0, len(f.pullErrors)+len(f.scanErrors))
+	for path, err := range f.pullErrors {
 		errors = append(errors, FileError{path, err})
 	}
+	f.pullErrorsMut.Unlock()
+	errors = append(errors, scanErrors...)
 	sort.Sort(fileErrorList(errors))
-	f.errorsMut.Unlock()
 	return errors
 }
 

lib/model/folder_sendrecv_test.go

@@ -98,8 +98,8 @@ func setUpSendReceiveFolder(model *Model) *sendReceiveFolder {
 		},
 
 		queue:         newJobQueue(),
-		errors:    make(map[string]string),
+		pullErrors:    make(map[string]string),
-		errorsMut: sync.NewMutex(),
+		pullErrorsMut: sync.NewMutex(),
 	}
 	f.fs = fs.NewMtimeFS(f.Filesystem(), db.NewNamespacedKV(model.db, "mtime"))
 

lib/model/folder_test.go

@@ -0,0 +1,150 @@
+// Copyright (C) 2018 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+package model
+
+import (
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/d4l3k/messagediff"
+	"github.com/syncthing/syncthing/lib/config"
+)
+
+type unifySubsCase struct {
+	in     []string // input to unifySubs
+	exists []string // paths that exist in the database
+	out    []string // expected output
+}
+
+func unifySubsCases() []unifySubsCase {
+	cases := []unifySubsCase{
+		{
+			// 0. trailing slashes are cleaned, known paths are just passed on
+			[]string{"foo/", "bar//"},
+			[]string{"foo", "bar"},
+			[]string{"bar", "foo"}, // the output is sorted
+		},
+		{
+			// 1. "foo/bar" gets trimmed as it's covered by foo
+			[]string{"foo", "bar/", "foo/bar/"},
+			[]string{"foo", "bar"},
+			[]string{"bar", "foo"},
+		},
+		{
+			// 2. "" gets simplified to the empty list; ie scan all
+			[]string{"foo", ""},
+			[]string{"foo"},
+			nil,
+		},
+		{
+			// 3. "foo/bar" is unknown, but it's kept
+			// because its parent is known
+			[]string{"foo/bar"},
+			[]string{"foo"},
+			[]string{"foo/bar"},
+		},
+		{
+			// 4. two independent known paths, both are kept
+			// "usr/lib" is not a prefix of "usr/libexec"
+			[]string{"usr/lib", "usr/libexec"},
+			[]string{"usr", "usr/lib", "usr/libexec"},
+			[]string{"usr/lib", "usr/libexec"},
+		},
+		{
+			// 5. "usr/lib" is a prefix of "usr/lib/exec"
+			[]string{"usr/lib", "usr/lib/exec"},
+			[]string{"usr", "usr/lib", "usr/libexec"},
+			[]string{"usr/lib"},
+		},
+		{
+			// 6. .stignore and .stfolder are special and are passed on
+			// verbatim even though they are unknown
+			[]string{config.DefaultMarkerName, ".stignore"},
+			[]string{},
+			[]string{config.DefaultMarkerName, ".stignore"},
+		},
+		{
+			// 7. but the presence of something else unknown forces an actual
+			// scan
+			[]string{config.DefaultMarkerName, ".stignore", "foo/bar"},
+			[]string{},
+			[]string{config.DefaultMarkerName, ".stignore", "foo"},
+		},
+		{
+			// 8. explicit request to scan all
+			nil,
+			[]string{"foo"},
+			nil,
+		},
+		{
+			// 9. empty list of subs
+			[]string{},
+			[]string{"foo"},
+			nil,
+		},
+		{
+			// 10. absolute path
+			[]string{"/foo"},
+			[]string{"foo"},
+			[]string{"foo"},
+		},
+	}
+
+	if runtime.GOOS == "windows" {
+		// Fixup path separators
+		for i := range cases {
+			for j, p := range cases[i].in {
+				cases[i].in[j] = filepath.FromSlash(p)
+			}
+			for j, p := range cases[i].exists {
+				cases[i].exists[j] = filepath.FromSlash(p)
+			}
+			for j, p := range cases[i].out {
+				cases[i].out[j] = filepath.FromSlash(p)
+			}
+		}
+	}
+
+	return cases
+}
+
+func unifyExists(f string, tc unifySubsCase) bool {
+	for _, e := range tc.exists {
+		if f == e {
+			return true
+		}
+	}
+	return false
+}
+
+func TestUnifySubs(t *testing.T) {
+	cases := unifySubsCases()
+	for i, tc := range cases {
+		exists := func(f string) bool {
+			return unifyExists(f, tc)
+		}
+		out := unifySubs(tc.in, exists)
+		if diff, equal := messagediff.PrettyDiff(tc.out, out); !equal {
+			t.Errorf("Case %d failed; got %v, expected %v, diff:\n%s", i, out, tc.out, diff)
+		}
+	}
+}
+
+func BenchmarkUnifySubs(b *testing.B) {
+	cases := unifySubsCases()
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		for _, tc := range cases {
+			exists := func(f string) bool {
+				return unifyExists(f, tc)
+			}
+			unifySubs(tc.in, exists)
+		}
+	}
+}

lib/model/model.go

@@ -8,7 +8,6 @@ package model
 
 import (
 	"bytes"
-	"context"
 	"crypto/tls"
 	"encoding/json"
 	"errors"
@@ -18,7 +17,6 @@ import (
 	"path/filepath"
 	"reflect"
 	"runtime"
-	"sort"
 	"strings"
 	stdsync "sync"
 	"time"
@@ -67,7 +65,7 @@ type service interface {
 	Serve()
 	Stop()
 	CheckHealth() error
-	PullErrors() []FileError
+	Errors() []FileError
 	WatchError() error
 
 	getState() (folderState, time.Time, error)
@@ -1981,264 +1979,6 @@ func (m *Model) ScanFolderSubdirs(folder string, subs []string) error {
 	return runner.Scan(subs)
 }
 
-func (m *Model) internalScanFolderSubdirs(ctx context.Context, folder string, subDirs []string, localFlags uint32) error {
-	m.fmut.RLock()
-	if err := m.checkFolderRunningLocked(folder); err != nil {
-		m.fmut.RUnlock()
-		return err
-	}
-	fset := m.folderFiles[folder]
-	folderCfg := m.folderCfgs[folder]
-	ignores := m.folderIgnores[folder]
-	runner := m.folderRunners[folder]
-	m.fmut.RUnlock()
-	mtimefs := fset.MtimeFS()
-
-	for i := range subDirs {
-		sub := osutil.NativeFilename(subDirs[i])
-
-		if sub == "" {
-			// A blank subdirs means to scan the entire folder. We can trim
-			// the subDirs list and go on our way.
-			subDirs = nil
-			break
-		}
-
-		subDirs[i] = sub
-	}
-
-	// Check if the ignore patterns changed as part of scanning this folder.
-	// If they did we should schedule a pull of the folder so that we
-	// request things we might have suddenly become unignored and so on.
-	oldHash := ignores.Hash()
-	defer func() {
-		if ignores.Hash() != oldHash {
-			l.Debugln("Folder", folder, "ignore patterns changed; triggering puller")
-			runner.IgnoresUpdated()
-		}
-	}()
-
-	if err := runner.CheckHealth(); err != nil {
-		return err
-	}
-
-	if err := ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) {
-		err = fmt.Errorf("loading ignores: %v", err)
-		runner.setError(err)
-		return err
-	}
-
-	// Clean the list of subitems to ensure that we start at a known
-	// directory, and don't scan subdirectories of things we've already
-	// scanned.
-	subDirs = unifySubs(subDirs, func(f string) bool {
-		_, ok := fset.Get(protocol.LocalDeviceID, f)
-		return ok
-	})
-
-	runner.setState(FolderScanning)
-
-	fchan := scanner.Walk(ctx, scanner.Config{
-		Folder:                folderCfg.ID,
-		Subs:                  subDirs,
-		Matcher:               ignores,
-		TempLifetime:          time.Duration(m.cfg.Options().KeepTemporariesH) * time.Hour,
-		CurrentFiler:          cFiler{m, folder},
-		Filesystem:            mtimefs,
-		IgnorePerms:           folderCfg.IgnorePerms,
-		AutoNormalize:         folderCfg.AutoNormalize,
-		Hashers:               m.numHashers(folder),
-		ShortID:               m.shortID,
-		ProgressTickIntervalS: folderCfg.ScanProgressIntervalS,
-		UseLargeBlocks:        folderCfg.UseLargeBlocks,
-		LocalFlags:            localFlags,
-	})
-
-	if err := runner.CheckHealth(); err != nil {
-		return err
-	}
-
-	batchFn := func(fs []protocol.FileInfo) error {
-		if err := runner.CheckHealth(); err != nil {
-			l.Debugf("Stopping scan of folder %s due to: %s", folderCfg.Description(), err)
-			return err
-		}
-		m.updateLocalsFromScanning(folder, fs)
-		return nil
-	}
-	// Resolve items which are identical with the global state.
-	if localFlags&protocol.FlagLocalReceiveOnly != 0 {
-		oldBatchFn := batchFn // can't reference batchFn directly (recursion)
-		batchFn = func(fs []protocol.FileInfo) error {
-			for i := range fs {
-				switch gf, ok := fset.GetGlobal(fs[i].Name); {
-				case !ok:
-					continue
-				case gf.IsEquivalentOptional(fs[i], false, false, protocol.FlagLocalReceiveOnly):
-					// What we have locally is equivalent to the global file.
-					fs[i].Version = fs[i].Version.Merge(gf.Version)
-					fallthrough
-				case fs[i].IsDeleted() && gf.IsReceiveOnlyChanged():
-					// Our item is deleted and the global item is our own
-					// receive only file. We can't delete file infos, so
-					// we just pretend it is a normal deleted file (nobody
-					// cares about that).
-					fs[i].LocalFlags &^= protocol.FlagLocalReceiveOnly
-				}
-			}
-			return oldBatchFn(fs)
-		}
-	}
-	batch := newFileInfoBatch(batchFn)
-
-	// Schedule a pull after scanning, but only if we actually detected any
-	// changes.
-	changes := 0
-	defer func() {
-		if changes > 0 {
-			runner.SchedulePull()
-		}
-	}()
-
-	for f := range fchan {
-		if err := batch.flushIfFull(); err != nil {
-			return err
-		}
-
-		batch.append(f)
-		changes++
-	}
-
-	if err := batch.flush(); err != nil {
-		return err
-	}
-
-	if len(subDirs) == 0 {
-		// If we have no specific subdirectories to traverse, set it to one
-		// empty prefix so we traverse the entire folder contents once.
-		subDirs = []string{""}
-	}
-
-	// Do a scan of the database for each prefix, to check for deleted and
-	// ignored files.
-	var toIgnore []db.FileInfoTruncated
-	ignoredParent := ""
-	pathSep := string(fs.PathSeparator)
-	for _, sub := range subDirs {
-		var iterError error
-
-		fset.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool {
-			f := fi.(db.FileInfoTruncated)
-
-			if err := batch.flushIfFull(); err != nil {
-				iterError = err
-				return false
-			}
-
-			if ignoredParent != "" && !strings.HasPrefix(f.Name, ignoredParent+pathSep) {
-				for _, f := range toIgnore {
-					l.Debugln("marking file as ignored", f)
-					nf := f.ConvertToIgnoredFileInfo(m.id.Short())
-					batch.append(nf)
-					changes++
-					if err := batch.flushIfFull(); err != nil {
-						iterError = err
-						return false
-					}
-				}
-				toIgnore = toIgnore[:0]
-				ignoredParent = ""
-			}
-
-			switch ignored := ignores.Match(f.Name).IsIgnored(); {
-			case !f.IsIgnored() && ignored:
-				// File was not ignored at last pass but has been ignored.
-				if f.IsDirectory() {
-					// Delay ignoring as a child might be unignored.
-					toIgnore = append(toIgnore, f)
-					if ignoredParent == "" {
-						// If the parent wasn't ignored already, set
-						// this path as the "highest" ignored parent
-						ignoredParent = f.Name
-					}
-					return true
-				}
-
-				l.Debugln("marking file as ignored", f)
-				nf := f.ConvertToIgnoredFileInfo(m.id.Short())
-				batch.append(nf)
-				changes++
-
-			case f.IsIgnored() && !ignored:
-				// Successfully scanned items are already un-ignored during
-				// the scan, so check whether it is deleted.
-				fallthrough
-			case !f.IsIgnored() && !f.IsDeleted() && !f.IsUnsupported():
-				// The file is not ignored, deleted or unsupported. Lets check if
-				// it's still here. Simply stat:ing it wont do as there are
-				// tons of corner cases (e.g. parent dir->symlink, missing
-				// permissions)
-				if !osutil.IsDeleted(mtimefs, f.Name) {
-					if ignoredParent != "" {
-						// Don't ignore parents of this not ignored item
-						toIgnore = toIgnore[:0]
-						ignoredParent = ""
-					}
-					return true
-				}
-				nf := protocol.FileInfo{
-					Name:       f.Name,
-					Type:       f.Type,
-					Size:       0,
-					ModifiedS:  f.ModifiedS,
-					ModifiedNs: f.ModifiedNs,
-					ModifiedBy: m.id.Short(),
-					Deleted:    true,
-					Version:    f.Version.Update(m.shortID),
-					LocalFlags: localFlags,
-				}
-				// We do not want to override the global version
-				// with the deleted file. Keeping only our local
-				// counter makes sure we are in conflict with any
-				// other existing versions, which will be resolved
-				// by the normal pulling mechanisms.
-				if f.ShouldConflict() {
-					nf.Version = nf.Version.DropOthers(m.shortID)
-				}
-
-				batch.append(nf)
-				changes++
-			}
-			return true
-		})
-
-		if iterError == nil && len(toIgnore) > 0 {
-			for _, f := range toIgnore {
-				l.Debugln("marking file as ignored", f)
-				nf := f.ConvertToIgnoredFileInfo(m.id.Short())
-				batch.append(nf)
-				changes++
-				if iterError = batch.flushIfFull(); iterError != nil {
-					break
-				}
-			}
-			toIgnore = toIgnore[:0]
-		}
-
-		if iterError != nil {
-			return iterError
-		}
-	}
-
-	if err := batch.flush(); err != nil {
-		return err
-	}
-
-	m.folderStatRef(folder).ScanCompleted()
-	runner.setState(FolderIdle)
-	return nil
-}
-
 func (m *Model) DelayScan(folder string, next time.Duration) {
 	m.fmut.Lock()
 	runner, ok := m.folderRunners[folder]
@@ -2351,13 +2091,13 @@ func (m *Model) State(folder string) (string, time.Time, error) {
 	return state.String(), changed, err
 }
 
-func (m *Model) PullErrors(folder string) ([]FileError, error) {
+func (m *Model) FolderErrors(folder string) ([]FileError, error) {
 	m.fmut.RLock()
 	defer m.fmut.RUnlock()
 	if err := m.checkFolderRunningLocked(folder); err != nil {
 		return nil, err
 	}
-	return m.folderRunners[folder].PullErrors(), nil
+	return m.folderRunners[folder].Errors(), nil
 }
 
 func (m *Model) WatchError(folder string) error {
@@ -2896,40 +2636,6 @@ func readOffsetIntoBuf(fs fs.Filesystem, file string, offset int64, buf []byte)
 	return err
 }
 
-// The exists function is expected to return true for all known paths
-// (excluding "" and ".")
-func unifySubs(dirs []string, exists func(dir string) bool) []string {
-	if len(dirs) == 0 {
-		return nil
-	}
-	sort.Strings(dirs)
-	if dirs[0] == "" || dirs[0] == "." || dirs[0] == string(fs.PathSeparator) {
-		return nil
-	}
-	prev := "./" // Anything that can't be parent of a clean path
-	for i := 0; i < len(dirs); {
-		dir, err := fs.Canonicalize(dirs[i])
-		if err != nil {
-			l.Debugf("Skipping %v for scan: %s", dirs[i], err)
-			dirs = append(dirs[:i], dirs[i+1:]...)
-			continue
-		}
-		if dir == prev || strings.HasPrefix(dir, prev+string(fs.PathSeparator)) {
-			dirs = append(dirs[:i], dirs[i+1:]...)
-			continue
-		}
-		parent := filepath.Dir(dir)
-		for parent != "." && parent != string(fs.PathSeparator) && !exists(parent) {
-			dir = parent
-			parent = filepath.Dir(dir)
-		}
-		dirs[i] = dir
-		prev = dir
-		i++
-	}
-	return dirs
-}
-
 // makeForgetUpdate takes an index update and constructs a download progress update
 // causing to forget any progress for files which we've just been sent.
 func makeForgetUpdate(files []protocol.FileInfo) []protocol.FileDownloadProgressUpdate {

lib/model/model_test.go

@@ -24,7 +24,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/d4l3k/messagediff"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/fs"
@@ -2352,140 +2351,6 @@ func benchmarkTree(b *testing.B, n1, n2 int) {
 	b.ReportAllocs()
 }
 
-type unifySubsCase struct {
-	in     []string // input to unifySubs
-	exists []string // paths that exist in the database
-	out    []string // expected output
-}
-
-func unifySubsCases() []unifySubsCase {
-	cases := []unifySubsCase{
-		{
-			// 0. trailing slashes are cleaned, known paths are just passed on
-			[]string{"foo/", "bar//"},
-			[]string{"foo", "bar"},
-			[]string{"bar", "foo"}, // the output is sorted
-		},
-		{
-			// 1. "foo/bar" gets trimmed as it's covered by foo
-			[]string{"foo", "bar/", "foo/bar/"},
-			[]string{"foo", "bar"},
-			[]string{"bar", "foo"},
-		},
-		{
-			// 2. "" gets simplified to the empty list; ie scan all
-			[]string{"foo", ""},
-			[]string{"foo"},
-			nil,
-		},
-		{
-			// 3. "foo/bar" is unknown, but it's kept
-			// because its parent is known
-			[]string{"foo/bar"},
-			[]string{"foo"},
-			[]string{"foo/bar"},
-		},
-		{
-			// 4. two independent known paths, both are kept
-			// "usr/lib" is not a prefix of "usr/libexec"
-			[]string{"usr/lib", "usr/libexec"},
-			[]string{"usr", "usr/lib", "usr/libexec"},
-			[]string{"usr/lib", "usr/libexec"},
-		},
-		{
-			// 5. "usr/lib" is a prefix of "usr/lib/exec"
-			[]string{"usr/lib", "usr/lib/exec"},
-			[]string{"usr", "usr/lib", "usr/libexec"},
-			[]string{"usr/lib"},
-		},
-		{
-			// 6. .stignore and .stfolder are special and are passed on
-			// verbatim even though they are unknown
-			[]string{config.DefaultMarkerName, ".stignore"},
-			[]string{},
-			[]string{config.DefaultMarkerName, ".stignore"},
-		},
-		{
-			// 7. but the presence of something else unknown forces an actual
-			// scan
-			[]string{config.DefaultMarkerName, ".stignore", "foo/bar"},
-			[]string{},
-			[]string{config.DefaultMarkerName, ".stignore", "foo"},
-		},
-		{
-			// 8. explicit request to scan all
-			nil,
-			[]string{"foo"},
-			nil,
-		},
-		{
-			// 9. empty list of subs
-			[]string{},
-			[]string{"foo"},
-			nil,
-		},
-		{
-			// 10. absolute path
-			[]string{"/foo"},
-			[]string{"foo"},
-			[]string{"foo"},
-		},
-	}
-
-	if runtime.GOOS == "windows" {
-		// Fixup path separators
-		for i := range cases {
-			for j, p := range cases[i].in {
-				cases[i].in[j] = filepath.FromSlash(p)
-			}
-			for j, p := range cases[i].exists {
-				cases[i].exists[j] = filepath.FromSlash(p)
-			}
-			for j, p := range cases[i].out {
-				cases[i].out[j] = filepath.FromSlash(p)
-			}
-		}
-	}
-
-	return cases
-}
-
-func unifyExists(f string, tc unifySubsCase) bool {
-	for _, e := range tc.exists {
-		if f == e {
-			return true
-		}
-	}
-	return false
-}
-
-func TestUnifySubs(t *testing.T) {
-	cases := unifySubsCases()
-	for i, tc := range cases {
-		exists := func(f string) bool {
-			return unifyExists(f, tc)
-		}
-		out := unifySubs(tc.in, exists)
-		if diff, equal := messagediff.PrettyDiff(tc.out, out); !equal {
-			t.Errorf("Case %d failed; got %v, expected %v, diff:\n%s", i, out, tc.out, diff)
-		}
-	}
-}
-
-func BenchmarkUnifySubs(b *testing.B) {
-	cases := unifySubsCases()
-	b.ReportAllocs()
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		for _, tc := range cases {
-			exists := func(f string) bool {
-				return unifyExists(f, tc)
-			}
-			unifySubs(tc.in, exists)
-		}
-	}
-}
-
 func TestIssue3028(t *testing.T) {
 	// Create two files that we'll delete, one with a name that is a prefix of the other.
 

lib/scanner/blockqueue.go

@@ -64,14 +64,14 @@ func HashFile(ctx context.Context, fs fs.Filesystem, path string, blockSize int,
 type parallelHasher struct {
 	fs      fs.Filesystem
 	workers int
-	outbox  chan<- protocol.FileInfo
+	outbox  chan<- ScanResult
 	inbox   <-chan protocol.FileInfo
 	counter Counter
 	done    chan<- struct{}
 	wg      sync.WaitGroup
 }
 
-func newParallelHasher(ctx context.Context, fs fs.Filesystem, workers int, outbox chan<- protocol.FileInfo, inbox <-chan protocol.FileInfo, counter Counter, done chan<- struct{}) {
+func newParallelHasher(ctx context.Context, fs fs.Filesystem, workers int, outbox chan<- ScanResult, inbox <-chan protocol.FileInfo, counter Counter, done chan<- struct{}) {
 	ph := &parallelHasher{
 		fs:      fs,
 		workers: workers,
@@ -122,7 +122,7 @@ func (ph *parallelHasher) hashFiles(ctx context.Context) {
 			}
 
 			select {
-			case ph.outbox <- f:
+			case ph.outbox <- ScanResult{File: f}:
 			case <-ctx.Done():
 				return
 			}

lib/scanner/walk.go

@@ -8,6 +8,8 @@ package scanner
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -61,7 +63,13 @@ type CurrentFiler interface {
 	CurrentFile(name string) (protocol.FileInfo, bool)
 }
 
-func Walk(ctx context.Context, cfg Config) chan protocol.FileInfo {
+type ScanResult struct {
+	File protocol.FileInfo
+	Err  error
+	Path string // to be set in case Err != nil and File == nil
+}
+
+func Walk(ctx context.Context, cfg Config) chan ScanResult {
 	w := walker{cfg}
 
 	if w.CurrentFiler == nil {
@@ -77,17 +85,23 @@ func Walk(ctx context.Context, cfg Config) chan protocol.FileInfo {
 	return w.walk(ctx)
 }
 
+var (
+	errUTF8Invalid       = errors.New("item is not in UTF8 encoding")
+	errUTF8Normalization = errors.New("item is not in the correct UTF8 normalization form")
+	errUTF8Conflict      = errors.New("item has UTF8 encoding conflict with another item")
+)
+
 type walker struct {
 	Config
 }
 
 // Walk returns the list of files found in the local folder by scanning the
 // file system. Files are blockwise hashed.
-func (w *walker) walk(ctx context.Context) chan protocol.FileInfo {
+func (w *walker) walk(ctx context.Context) chan ScanResult {
 	l.Debugln("Walk", w.Subs, w.Matcher)
 
 	toHashChan := make(chan protocol.FileInfo)
-	finishedChan := make(chan protocol.FileInfo)
+	finishedChan := make(chan ScanResult)
 
 	// A routine which walks the filesystem tree, and sends files which have
 	// been modified to the counter routine.
@@ -182,7 +196,7 @@ func (w *walker) walk(ctx context.Context) chan protocol.FileInfo {
 	return finishedChan
 }
 
-func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protocol.FileInfo) fs.WalkFunc {
+func (w *walker) walkAndHashFiles(ctx context.Context, toHashChan chan<- protocol.FileInfo, finishedChan chan<- ScanResult) fs.WalkFunc {
 	now := time.Now()
 	ignoredParent := ""
 
@@ -202,7 +216,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 		}
 
 		if err != nil {
-			l.Debugln("error:", path, info, err)
+			w.handleError(ctx, "scan", path, err, finishedChan)
 			return skip
 		}
 
@@ -225,7 +239,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 		}
 
 		if !utf8.ValidString(path) {
-			l.Warnf("File name %q is not in UTF8 encoding; skipping.", path)
+			w.handleError(ctx, "scan", path, errUTF8Invalid, finishedChan)
 			return skip
 		}
 
@@ -244,7 +258,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 
 		if ignoredParent == "" {
 			// parent isn't ignored, nothing special
-			return w.handleItem(ctx, path, fchan, dchan, skip)
+			return w.handleItem(ctx, path, toHashChan, finishedChan, skip)
 		}
 
 		// Part of current path below the ignored (potential) parent
@@ -253,17 +267,17 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 		// ignored path isn't actually a parent of the current path
 		if rel == path {
 			ignoredParent = ""
-			return w.handleItem(ctx, path, fchan, dchan, skip)
+			return w.handleItem(ctx, path, toHashChan, finishedChan, skip)
 		}
 
 		// The previously ignored parent directories of the current, not
 		// ignored path need to be handled as well.
-		if err = w.handleItem(ctx, ignoredParent, fchan, dchan, skip); err != nil {
+		if err = w.handleItem(ctx, ignoredParent, toHashChan, finishedChan, skip); err != nil {
 			return err
 		}
 		for _, name := range strings.Split(rel, string(fs.PathSeparator)) {
 			ignoredParent = filepath.Join(ignoredParent, name)
-			if err = w.handleItem(ctx, ignoredParent, fchan, dchan, skip); err != nil {
+			if err = w.handleItem(ctx, ignoredParent, toHashChan, finishedChan, skip); err != nil {
 				return err
 			}
 		}
@@ -273,21 +287,23 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco
 	}
 }
 
-func (w *walker) handleItem(ctx context.Context, path string, fchan, dchan chan protocol.FileInfo, skip error) error {
+func (w *walker) handleItem(ctx context.Context, path string, toHashChan chan<- protocol.FileInfo, finishedChan chan<- ScanResult, skip error) error {
 	info, err := w.Filesystem.Lstat(path)
 	// An error here would be weird as we've already gotten to this point, but act on it nonetheless
 	if err != nil {
+		w.handleError(ctx, "scan", path, err, finishedChan)
 		return skip
 	}
 
-	path, shouldSkip := w.normalizePath(path, info)
+	path, err = w.normalizePath(path, info)
-	if shouldSkip {
+	if err != nil {
+		w.handleError(ctx, "normalizing path", path, err, finishedChan)
 		return skip
 	}
 
 	switch {
 	case info.IsSymlink():
-		if err := w.walkSymlink(ctx, path, dchan); err != nil {
+		if err := w.walkSymlink(ctx, path, finishedChan); err != nil {
 			return err
 		}
 		if info.IsDir() {
@@ -297,16 +313,16 @@ func (w *walker) handleItem(ctx context.Context, path string, fchan, dchan chan
 		return nil
 
 	case info.IsDir():
-		err = w.walkDir(ctx, path, info, dchan)
+		err = w.walkDir(ctx, path, info, finishedChan)
 
 	case info.IsRegular():
-		err = w.walkRegular(ctx, path, info, fchan)
+		err = w.walkRegular(ctx, path, info, toHashChan)
 	}
 
 	return err
 }
 
-func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileInfo, fchan chan protocol.FileInfo) error {
+func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileInfo, toHashChan chan<- protocol.FileInfo) error {
 	curFile, hasCurFile := w.CurrentFiler.CurrentFile(relPath)
 
 	blockSize := protocol.MinBlockSize
@@ -352,7 +368,7 @@ func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileIn
 	l.Debugln("to hash:", relPath, f)
 
 	select {
-	case fchan <- f:
+	case toHashChan <- f:
 	case <-ctx.Done():
 		return ctx.Err()
 	}
@@ -360,7 +376,7 @@ func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileIn
 	return nil
 }
 
-func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, dchan chan protocol.FileInfo) error {
+func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, finishedChan chan<- ScanResult) error {
 	curFile, hasCurFile := w.CurrentFiler.CurrentFile(relPath)
 
 	f, _ := CreateFileInfo(info, relPath, nil)
@@ -384,7 +400,7 @@ func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo,
 	l.Debugln("dir:", relPath, f)
 
 	select {
-	case dchan <- f:
+	case finishedChan <- ScanResult{File: f}:
 	case <-ctx.Done():
 		return ctx.Err()
 	}
@@ -394,7 +410,7 @@ func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo,
 
 // walkSymlink returns nil or an error, if the error is of the nature that
 // it should stop the entire walk.
-func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan protocol.FileInfo) error {
+func (w *walker) walkSymlink(ctx context.Context, relPath string, finishedChan chan<- ScanResult) error {
 	// Symlinks are not supported on Windows. We ignore instead of returning
 	// an error.
 	if runtime.GOOS == "windows" {
@@ -408,7 +424,7 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan pro
 
 	target, err := w.Filesystem.ReadSymlink(relPath)
 	if err != nil {
-		l.Debugln("readlink error:", relPath, err)
+		w.handleError(ctx, "reading link:", relPath, err, finishedChan)
 		return nil
 	}
 
@@ -439,7 +455,7 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan pro
 	l.Debugln("symlink changedb:", relPath, f)
 
 	select {
-	case dchan <- f:
+	case finishedChan <- ScanResult{File: f}:
 	case <-ctx.Done():
 		return ctx.Err()
 	}
@@ -449,7 +465,7 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan pro
 
 // normalizePath returns the normalized relative path (possibly after fixing
 // it on disk), or skip is true.
-func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, skip bool) {
+func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, err error) {
 	if runtime.GOOS == "darwin" {
 		// Mac OS X file names should always be NFD normalized.
 		normPath = norm.NFD.String(path)
@@ -462,14 +478,13 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string,
 
 	if path == normPath {
 		// The file name is already normalized: nothing to do
-		return path, false
+		return path, nil
 	}
 
 	if !w.AutoNormalize {
 		// We're not authorized to do anything about it, so complain and skip.
 
-		l.Warnf("File name %q is not in the correct UTF8 normalization form; skipping.", path)
+		return "", errUTF8Normalization
-		return "", true
 	}
 
 	// We will attempt to normalize it.
@@ -477,11 +492,12 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string,
 	if fs.IsNotExist(err) {
 		// Nothing exists with the normalized filename. Good.
 		if err = w.Filesystem.Rename(path, normPath); err != nil {
-			l.Infof(`Error normalizing UTF8 encoding of file "%s": %v`, path, err)
+			return "", err
-			return "", true
 		}
 		l.Infof(`Normalized UTF8 encoding of file name "%s".`, path)
-	} else if w.Filesystem.SameFile(info, normInfo) {
+		return normPath, nil
+	}
+	if w.Filesystem.SameFile(info, normInfo) {
 		// With some filesystems (ZFS), if there is an un-normalized path and you ask whether the normalized
 		// version exists, it responds with true. Therefore we need to check fs.SameFile as well.
 		// In this case, a call to Rename won't do anything, so we have to rename via a temp file.
@@ -491,23 +507,19 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string,
 
 		tempPath := fs.TempNameWithPrefix(normPath, "")
 		if err = w.Filesystem.Rename(path, tempPath); err != nil {
-			l.Infof(`Error during normalizing UTF8 encoding of file "%s" (renamed to "%s"): %v`, path, tempPath, err)
+			return "", err
-			return "", true
 		}
 		if err = w.Filesystem.Rename(tempPath, normPath); err != nil {
 			// I don't ever expect this to happen, but if it does, we should probably tell our caller that the normalized
 			// path is the temp path: that way at least the user's data still gets synced.
 			l.Warnf(`Error renaming "%s" to "%s" while normalizating UTF8 encoding: %v. You will want to rename this file back manually`, tempPath, normPath, err)
-			return tempPath, false
+			return tempPath, nil
+		}
+		return normPath, nil
 	}
-	} else {
 	// There is something already in the way at the normalized
 	// file name.
-		l.Infof(`File "%s" path has UTF8 encoding conflict with another file; ignoring.`, path)
+	return "", errUTF8Conflict
-		return "", true
-	}
-
-	return normPath, false
 }
 
 // updateFileInfo updates walker specific members of protocol.FileInfo that do not depend on type
@@ -522,6 +534,16 @@ func (w *walker) updateFileInfo(file, curFile protocol.FileInfo) protocol.FileIn
 	file.LocalFlags = w.LocalFlags
 	return file
 }
+func (w *walker) handleError(ctx context.Context, context, path string, err error, finishedChan chan<- ScanResult) {
+	l.Infof("Scanner (folder %s, file %q): %s: %v", w.Folder, path, context, err)
+	select {
+	case finishedChan <- ScanResult{
+		Err:  fmt.Errorf("%s: %s", context, err.Error()),
+		Path: path,
+	}:
+	case <-ctx.Done():
+	}
+}
 
 // A byteCounter gets bytes added to it via Update() and then provides the
 // Total() and one minute moving average Rate() in bytes per second.

lib/scanner/walk_test.go

@@ -74,7 +74,10 @@ func TestWalkSub(t *testing.T) {
 	})
 	var files []protocol.FileInfo
 	for f := range fchan {
-		files = append(files, f)
+		if f.Err != nil {
+			t.Errorf("Error while scanning %v: %v", f.Err, f.Path)
+		}
+		files = append(files, f.File)
 	}
 
 	// The directory contains two files, where one is ignored from a higher
@@ -107,7 +110,10 @@ func TestWalk(t *testing.T) {
 
 	var tmp []protocol.FileInfo
 	for f := range fchan {
-		tmp = append(tmp, f)
+		if f.Err != nil {
+			t.Errorf("Error while scanning %v: %v", f.Err, f.Path)
+		}
+		tmp = append(tmp, f.File)
 	}
 	sort.Sort(fileList(tmp))
 	files := fileList(tmp).testfiles()
@@ -246,8 +252,9 @@ func TestNormalization(t *testing.T) {
 
 func TestIssue1507(t *testing.T) {
 	w := &walker{}
-	c := make(chan protocol.FileInfo, 100)
+	h := make(chan protocol.FileInfo, 100)
-	fn := w.walkAndHashFiles(context.TODO(), c, c)
+	f := make(chan ScanResult, 100)
+	fn := w.walkAndHashFiles(context.TODO(), h, f)
 
 	fn("", nil, protocol.ErrClosed)
 }
@@ -471,7 +478,9 @@ func walkDir(fs fs.Filesystem, dir string, cfiler CurrentFiler, matcher *ignore.
 
 	var tmp []protocol.FileInfo
 	for f := range fchan {
-		tmp = append(tmp, f)
+		if f.Err == nil {
+			tmp = append(tmp, f.File)
+		}
 	}
 	sort.Sort(fileList(tmp))
 
@@ -580,7 +589,11 @@ func TestStopWalk(t *testing.T) {
 	dirs := 0
 	files := 0
 	for {
-		f := <-fchan
+		res := <-fchan
+		if res.Err != nil {
+			t.Errorf("Error while scanning %v: %v", res.Err, res.Path)
+		}
+		f := res.File
 		t.Log("Scanned", f)
 		if f.IsDirectory() {
 			if len(f.Name) == 0 || f.Permissions == 0 {
@@ -710,7 +723,10 @@ func TestIssue4841(t *testing.T) {
 
 	var files []protocol.FileInfo
 	for f := range fchan {
-		files = append(files, f)
+		if f.Err != nil {
+			t.Errorf("Error while scanning %v: %v", f.Err, f.Path)
+		}
+		files = append(files, f.File)
 	}
 	sort.Sort(fileList(files))