Ignore FS errors from stat() in some cases. (#10021)

(for #9901)

The earlier FS code ignored errors in a couple of situations. Recent changes inadvertently stopped ignoring them. This change restores the earlier behavior (and adds logging of the errors).

Author: ericsnowcurrently

news/2 Fixes/9901.md

@@ -0,0 +1 @@
+Ignore errors coming from stat(), where appropriate.

src/client/common/platform/fileSystem.ts

@@ -9,6 +9,7 @@ import { injectable } from 'inversify';
 import { promisify } from 'util';
 import * as vscode from 'vscode';
 import '../../common/extensions';
+import { traceError } from '../logger';
 import { createDeferred } from '../utils/async';
 import { isFileNotFoundError, isNoPermissionsError } from './errors';
 import { FileSystemPaths, FileSystemPathUtils } from './fs-paths';
@@ -213,9 +214,15 @@ export class RawFileSystem implements IRawFileSystem {
         const files = await this.fsExtra.readdir(dirname);
         const promises = files.map(async basename => {
             const filename = this.paths.join(dirname, basename);
-            // Note that this follows symlinks (while still preserving
-            // the Symlink flag).
-            const fileType = await this.getFileType(filename);
+            let fileType: FileType;
+            try {
+                // Note that getFileType() follows symlinks (while still
+                // preserving the Symlink flag).
+                fileType = await this.getFileType(filename);
+            } catch (err) {
+                traceError(`failure while getting file type for "${filename}"`, err);
+                fileType = FileType.Unknown;
+            }
             return [filename, fileType] as [string, FileType];
         });
         return Promise.all(promises);
@@ -352,7 +359,8 @@ export class FileSystemUtils implements IFileSystemUtils {
             if (isFileNotFoundError(err)) {
                 return false;
             }
-            throw err;
+            traceError(`stat() failed for "${filename}"`, err);
+            return false;
         }
 
         if (fileType === undefined) {

src/test/common/platform/filesystem.functional.test.ts

@@ -678,6 +678,31 @@ suite('FileSystem - raw', () => {
 
             await expect(promise).to.eventually.be.rejected;
         });
+
+        test('ignores errors from getFileType()', async function() {
+            if (WINDOWS) {
+                // tslint:disable-next-line:no-invalid-this
+                this.skip();
+            }
+            const dirname = await fix.createDirectory('x/y/z');
+            const file1 = await fix.createFile('x/y/z/__init__.py', '');
+            const file2 = await fix.createFile('x/y/z/spam.py', '...');
+            const file3 = await fix.createFile('x/y/z/eggs.py', '...');
+            await fs.chmod(dirname, 0o400);
+
+            let entries: [string, FileType][];
+            try {
+                entries = await fileSystem.listdir(dirname);
+            } finally {
+                await fs.chmod(dirname, 0o755);
+            }
+
+            expect(entries.sort()).to.deep.equal([
+                [file1, FileType.Unknown],
+                [file3, FileType.Unknown],
+                [file2, FileType.Unknown]
+            ]);
+        });
     });
 
     // non-async

src/test/common/platform/filesystem.test.ts

@@ -17,7 +17,7 @@ import {
 // prettier-ignore
 import {
     assertDoesNotExist, DOES_NOT_EXIST, FSFixture,
-    SUPPORTS_SOCKETS, SUPPORTS_SYMLINKS
+    SUPPORTS_SOCKETS, SUPPORTS_SYMLINKS, WINDOWS
 } from './utils';
 
 // Note: all functional tests that do not trigger the VS Code "fs" API
@@ -242,6 +242,25 @@ suite('FileSystem - utils', () => {
 
             expect(exists).to.equal(false);
         });
+
+        test('failure in stat()', async function() {
+            if (WINDOWS) {
+                // tslint:disable-next-line:no-invalid-this
+                this.skip();
+            }
+            const dirname = await fix.createDirectory('x/y/z');
+            const filename = await fix.createFile('x/y/z/spam.py', '...');
+            await fsextra.chmod(dirname, 0o400);
+
+            let exists: boolean;
+            try {
+                exists = await utils.fileExists(filename);
+            } finally {
+                await fsextra.chmod(dirname, 0o755);
+            }
+
+            expect(exists).to.equal(false);
+        });
     });
 
     suite('directoryExists', () => {
@@ -282,6 +301,25 @@ suite('FileSystem - utils', () => {
 
             expect(exists).to.equal(false);
         });
+
+        test('failure in stat()', async function() {
+            if (WINDOWS) {
+                // tslint:disable-next-line:no-invalid-this
+                this.skip();
+            }
+            const parentdir = await fix.createDirectory('x/y/z');
+            const dirname = await fix.createDirectory('x/y/z/spam');
+            await fsextra.chmod(parentdir, 0o400);
+
+            let exists: boolean;
+            try {
+                exists = await utils.fileExists(dirname);
+            } finally {
+                await fsextra.chmod(parentdir, 0o755);
+            }
+
+            expect(exists).to.equal(false);
+        });
     });
 
     suite('getSubDirectories', () => {

src/test/common/platform/filesystem.unit.test.ts

@@ -597,6 +597,38 @@ suite('Raw FileSystem', () => {
             await expect(promise).to.eventually.be.rejected;
             verifyAll();
         });
+
+        test('ignores errors from getFileType()', async () => {
+            const dirname = 'x/y/z';
+            const names = [
+                // These match the items in "expected".
+                '__init__.py',
+                'spam.py',
+                'eggs.py'
+            ];
+            const expected: [string, FileType][] = [
+                ['x/y/z/__init__.py', FileType.File],
+                ['x/y/z/spam.py', FileType.File],
+                ['x/y/z/eggs.py', FileType.Unknown]
+            ];
+            raw.setup(r => r.readdir(dirname)) // expect the specific filename
+                .returns(() => Promise.resolve(names));
+            names.forEach((name, i) => {
+                const [filename, filetype] = expected[i];
+                raw.setup(r => r.join(dirname, name)) // expect the specific filename
+                    .returns(() => filename);
+                if (filetype === FileType.Unknown) {
+                    raw.setup(r => r.lstat(filename)) // expect the specific filename
+                        .throws(new Error('oops!'));
+                } else {
+                    setupForFileType(filename, filetype);
+                }
+            });
+
+            const entries = await filesystem.listdir(dirname);
+
+            expect(entries.sort()).to.deep.equal(expected.sort());
+        });
     });
 
     suite('readTextSync', () => {
@@ -781,15 +813,14 @@ suite('FileSystemUtils', () => {
             verifyAll();
         });
 
-        test('fails if stat fails', async () => {
+        test('ignores errors from stat()', async () => {
             const filename = 'x/y/z/spam.py';
-            const err = new Error('oops!');
-            deps.setup(d => d.stat(filename)) // There was a problem while stat'ing the file.
-                .throws(err);
+            deps.setup(d => d.stat(filename)) // It's broken.
+                .returns(() => Promise.reject(new Error('oops!')));
 
-            const promise = utils.pathExists(filename);
+            const exists = await utils.pathExists(filename);
 
-            await expect(promise).to.eventually.be.rejected;
+            expect(exists).to.equal(false);
             verifyAll();
         });