Merge pull request #2335 from posit-dev/mnv-keep-quiet-symlink-errs

Be quiet for symlink walking errors

internal/bundles/bundle_test.go

@@ -396,6 +396,8 @@ func (s *BundlerSuite) TestNewBundleFromDirectorySymlinks() {
 	}, s.getTarFileNames(dest))
 }
 
+// We log the issues with symbolic links but not return them to not polute the user with error notifications
+// when another piece of software is dealing with the same directory.
 func (s *BundlerSuite) TestNewBundleFromDirectoryMissingSymlinkTarget() {
 	// afero's MemFs doesn't have symlink support, so we
 	// are using a fixture directory under ./testdata.
@@ -407,6 +409,6 @@ func (s *BundlerSuite) TestNewBundleFromDirectoryMissingSymlinkTarget() {
 	bundler, err := NewBundler(dirPath, NewManifest(), nil, log)
 	s.Nil(err)
 	manifest, err := bundler.CreateBundle(dest)
-	s.ErrorIs(err, os.ErrNotExist)
-	s.Nil(manifest)
+	s.NoError(err)
+	s.NotNil(manifest)
 }

internal/bundles/matcher/walker.go

@@ -85,6 +85,10 @@ func (i *matchingWalker) Walk(base util.AbsolutePath, fn util.AbsoluteWalkFunc)
 			i.log.Warn("permission error; skipping", "path", path)
 			return nil
 		}
+		if err != nil {
+			i.log.Warn("Unknown error while accessing file", "path", path, "error", err.Error())
+			return nil
+		}
 		return fn(path, info, err)
 	})
 }

internal/bundles/matcher/walker_test.go

@@ -3,15 +3,18 @@ package matcher
 // Copyright (C) 2023 by Posit Software, PBC.
 
 import (
+	"errors"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/logging/loggingtest"
 	"github.com/posit-dev/publisher/internal/util"
 	"github.com/posit-dev/publisher/internal/util/utiltest"
 	"github.com/spf13/afero"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -136,6 +139,30 @@ func (s *WalkerSuite) TestWalkPermissionErr() {
 	s.Equal([]string{"."}, seen)
 }
 
+func (s *WalkerSuite) TestWalkErr_Logged() {
+	// Errors while walking through files are logged but won't halt the current process
+	afs := utiltest.NewMockFs()
+	baseDir := s.cwd.WithFs(afs)
+
+	fileInfo, err := s.cwd.Stat()
+	s.NoError(err)
+	afs.On("Stat", baseDir.String()).Return(fileInfo, errors.New("batteries not included"))
+
+	log := loggingtest.NewMockLogger()
+	log.On("Warn", "Unknown error while accessing file", "path", mock.Anything, "error", mock.Anything).Return()
+
+	w, err := NewMatchingWalker([]string{"*"}, s.cwd, log)
+	s.NoError(err)
+	s.NotNil(w)
+
+	err = w.Walk(baseDir, func(path util.AbsolutePath, info fs.FileInfo, err error) error {
+		s.NoError(err)
+		return nil
+	})
+	s.NoError(err)
+	log.AssertExpectations(s.T())
+}
+
 func (s *WalkerSuite) TestWalkSubdirectory() {
 	baseDir := s.cwd.Join("test", "dir")
 	err := baseDir.MkdirAll(0777)

internal/services/api/files/services.go

@@ -52,7 +52,8 @@ func (s filesService) GetFile(p util.AbsolutePath, matchList matcher.MatchList)
 				s.log.Warn("permission error; skipping", "path", path)
 				return nil
 			} else {
-				return err
+				s.log.Warn("Unknown error while accesing file", "path", path)
+				return nil
 			}
 		}
 		match := matchList.Match(path)

internal/util/symlink_walker.go

@@ -3,7 +3,6 @@ package util
 // Copyright (C) 2023 by Posit Software, PBC.
 
 import (
-	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -40,13 +39,20 @@ func (w *symlinkWalker) visit(fn AbsoluteWalkFunc) AbsoluteWalkFunc {
 		if info.Mode().Type()&os.ModeSymlink != 0 {
 			w.log.Info("Following symlink", "path", path)
 			linkTarget, err := filepath.EvalSymlinks(path.String())
-			targetPath := NewPath(linkTarget, path.Fs())
 			if err != nil {
-				return fmt.Errorf("error following symlink %s: %w", path, err)
+				// There's software that creates symbolic links to files which do not exist,
+				// like Emacs which creates symbolic files for unsaved modifications to files.
+				// We'll log the behavior but not return errors to not polute the user with error notifications
+				// when another piece of software is dealing with the same directory.
+				w.log.Warn("Error following symlink, ignoring file", "filepath", path, "error", err.Error())
+				return nil
 			}
+
+			targetPath := NewPath(linkTarget, path.Fs())
 			targetInfo, err := targetPath.Stat()
 			if err != nil {
-				return fmt.Errorf("error getting target info for symlink %s: %w", targetPath, err)
+				w.log.Warn("Error getting info for symlink", "filepath", targetPath, "error", err.Error())
+				return nil
 			}
 			// Visit symlink target info but use the path to the link.
 			err = w.visit(fn)(path, targetInfo, nil)

internal/util/symlink_walker_test.go

@@ -5,13 +5,14 @@ package util
 import (
 	"errors"
 	"io/fs"
-	"os"
 	"path/filepath"
 	"runtime"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/logging/loggingtest"
 	"github.com/posit-dev/publisher/internal/util/utiltest"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/mock"
@@ -116,17 +117,27 @@ func (s *SymlinkWalkerSuite) TestNewBundleFromDirectorySymlinks() {
 	}, fileList)
 }
 
+// We log the issues with symbolic links but not return them to not polute the user with error notifications
+// when another piece of software is dealing with the same directory.
 func (s *SymlinkWalkerSuite) TestNewBundleFromDirectoryMissingSymlinkTarget() {
 	// afero's MemFs doesn't have symlink support, so we
 	// are using a fixture directory under ./testdata.
 	realFS := afero.NewOsFs()
 	dirPath := NewAbsolutePath(s.cwd.String(), realFS).Join("testdata", "symlink_test", "link_target_missing")
-	log := logging.New()
+	log := loggingtest.NewMockLogger()
+
+	symlinkPathMatcher := mock.MatchedBy(func(path AbsolutePath) bool {
+		return strings.Contains(path.String(), "badlink")
+	})
+
+	log.On("Info", "Following symlink", "path", symlinkPathMatcher).Return()
+	log.On("Warn", "Error following symlink, ignoring file", "filepath", symlinkPathMatcher, "error", mock.Anything).Return()
 
 	underlyingWalker := &FSWalker{}
 	walker := NewSymlinkWalker(underlyingWalker, log)
 	err := walker.Walk(dirPath, func(path AbsolutePath, info fs.FileInfo, err error) error {
 		return nil
 	})
-	s.ErrorIs(err, os.ErrNotExist)
+	s.NoError(err)
+	log.AssertExpectations(s.T())
 }