From cb671004a3d3fa9b0eda5cc276dbe47be8ff13f9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 10 Feb 2020 11:18:17 -0700 Subject: [PATCH 1/3] Ignore errors in listdir() from getFileType(). --- src/client/common/platform/fileSystem.ts | 13 ++++++-- .../platform/filesystem.functional.test.ts | 25 +++++++++++++++ .../common/platform/filesystem.unit.test.ts | 32 +++++++++++++++++++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 0217163ce37c..fd764bd04627 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/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); diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 3c5497ac88f3..fda385686562 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/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 diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 1764add2cd30..2a9f7ba66182 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/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', () => { From fad8f671799edbd7bf73ef0ca60b7517ecb385b4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 10 Feb 2020 11:37:55 -0700 Subject: [PATCH 2/3] Ignore stat() failures in pathExists(). --- src/client/common/platform/fileSystem.ts | 3 +- src/test/common/platform/filesystem.test.ts | 40 ++++++++++++++++++- .../common/platform/filesystem.unit.test.ts | 11 +++-- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index fd764bd04627..dcef489a2c02 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -359,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) { diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts index 3b4c9a69f2b9..5346ba851f51 100644 --- a/src/test/common/platform/filesystem.test.ts +++ b/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', () => { diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 2a9f7ba66182..b5148cbd2047 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -813,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(); }); From cbb13e27206ff312d53988dd6ceb8b3c994be486 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 10 Feb 2020 11:39:34 -0700 Subject: [PATCH 3/3] Add a NEWS entry. --- news/2 Fixes/9901.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/9901.md diff --git a/news/2 Fixes/9901.md b/news/2 Fixes/9901.md new file mode 100644 index 000000000000..3f04054b0926 --- /dev/null +++ b/news/2 Fixes/9901.md @@ -0,0 +1 @@ +Ignore errors coming from stat(), where appropriate.