Skip to content

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

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/9901.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ignore errors coming from stat(), where appropriate.
16 changes: 12 additions & 4 deletions src/client/common/platform/fileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
25 changes: 25 additions & 0 deletions src/test/common/platform/filesystem.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 39 additions & 1 deletion src/test/common/platform/filesystem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
43 changes: 37 additions & 6 deletions src/test/common/platform/filesystem.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
});

Expand Down