Skip to content

Commit 3e81a1d

Browse files
[CP-stable][tool] make ErrorHandlingFileSystem.deleteIfExists catch error code 3 (ERROR_PATH_NOT_FOUND on Windows) (flutter#150787)
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? flutter#150736 ### Changelog Description: Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples Fixes some CLI tool crashes due to trying to delete a file that no longer exists. ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch) When running certain commands, such as `flutter run` or `flutter build`, users get a lengthy crash message including the full contents of a FileSystemException. ### Workaround: Is there a workaround for this issue? Attempt to run the command again, or try again after running `flutter clean`. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? N/A. This issue is due to races within the user's file system (a file getting deleted or quarantined after being confirmed to exist but before the tool could delete it.
1 parent 0054d43 commit 3e81a1d

File tree

2 files changed

+39
-13
lines changed

2 files changed

+39
-13
lines changed

packages/flutter_tools/lib/src/base/error_handling_io.dart

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,13 @@ import 'platform.dart';
2121
// ToolExit and a message that is more clear than the FileSystemException by
2222
// itself.
2323

24-
/// On windows this is error code 2: ERROR_FILE_NOT_FOUND, and on
24+
/// On Windows this is error code 2: ERROR_FILE_NOT_FOUND, and on
2525
/// macOS/Linux it is error code 2/ENOENT: No such file or directory.
26-
const int kSystemCannotFindFile = 2;
26+
const int kSystemCodeCannotFindFile = 2;
27+
28+
/// On Windows this error is 3: ERROR_PATH_NOT_FOUND, and on
29+
/// macOS/Linux, it is error code 3/ESRCH: No such process.
30+
const int kSystemCodePathNotFound = 3;
2731

2832
/// A [FileSystem] that throws a [ToolExit] on certain errors.
2933
///
@@ -72,22 +76,26 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem {
7276
/// This method should be preferred to checking if it exists and
7377
/// then deleting, because it handles the edge case where the file or directory
7478
/// is deleted by a different program between the two calls.
75-
static bool deleteIfExists(FileSystemEntity file, {bool recursive = false}) {
76-
if (!file.existsSync()) {
79+
static bool deleteIfExists(FileSystemEntity entity, {bool recursive = false}) {
80+
if (!entity.existsSync()) {
7781
return false;
7882
}
7983
try {
80-
file.deleteSync(recursive: recursive);
84+
entity.deleteSync(recursive: recursive);
8185
} on FileSystemException catch (err) {
8286
// Certain error codes indicate the file could not be found. It could have
8387
// been deleted by a different program while the tool was running.
8488
// if it still exists, the file likely exists on a read-only volume.
85-
if (err.osError?.errorCode != kSystemCannotFindFile || _noExitOnFailure) {
89+
// This check will falsely match "3/ESRCH: No such process" on Linux/macOS,
90+
// but this should be fine since this code should never come up here.
91+
final bool codeCorrespondsToPathOrFileNotFound = err.osError?.errorCode == kSystemCodeCannotFindFile ||
92+
err.osError?.errorCode == kSystemCodePathNotFound;
93+
if (!codeCorrespondsToPathOrFileNotFound || _noExitOnFailure) {
8694
rethrow;
8795
}
88-
if (file.existsSync()) {
96+
if (entity.existsSync()) {
8997
throwToolExit(
90-
'The Flutter tool tried to delete the file or directory ${file.path} but was '
98+
'The Flutter tool tried to delete the file or directory ${entity.path} but was '
9199
"unable to. This may be due to the file and/or project's location on a read-only "
92100
'volume. Consider relocating the project and trying again',
93101
);
@@ -104,7 +112,7 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem {
104112
return _runSync(() => directory(delegate.currentDirectory), platform: _platform);
105113
} on FileSystemException catch (err) {
106114
// Special handling for OS error 2 for current directory only.
107-
if (err.osError?.errorCode == kSystemCannotFindFile) {
115+
if (err.osError?.errorCode == kSystemCodeCannotFindFile) {
108116
throwToolExit(
109117
'Unable to read current working directory. This can happen if the directory the '
110118
'Flutter tool was run from was moved or deleted.'

packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,24 @@ void main() {
9898
}, throwsFileSystemException());
9999
});
100100

101+
testWithoutContext('deleteIfExists throws tool exit if the path is not found on Windows', () {
102+
final FileExceptionHandler exceptionHandler = FileExceptionHandler();
103+
final ErrorHandlingFileSystem fileSystem = ErrorHandlingFileSystem(
104+
delegate: MemoryFileSystem.test(opHandle: exceptionHandler.opHandle),
105+
platform: windowsPlatform,
106+
);
107+
final File file = fileSystem.file(fileSystem.path.join('directory', 'file'))
108+
..createSync(recursive: true);
109+
110+
exceptionHandler.addError(
111+
file,
112+
FileSystemOp.delete,
113+
FileSystemException('', file.path, const OSError('', 2)),
114+
);
115+
116+
expect(() => ErrorHandlingFileSystem.deleteIfExists(file), throwsToolExit());
117+
});
118+
101119
group('throws ToolExit on Windows', () {
102120
const int kDeviceFull = 112;
103121
const int kUserMappedSectionOpened = 1224;
@@ -571,14 +589,14 @@ void main() {
571589

572590
testWithoutContext('When the current working directory disappears', () async {
573591
final ErrorHandlingFileSystem fileSystem = ErrorHandlingFileSystem(
574-
delegate: ThrowsOnCurrentDirectoryFileSystem(kSystemCannotFindFile),
592+
delegate: ThrowsOnCurrentDirectoryFileSystem(kSystemCodeCannotFindFile),
575593
platform: linuxPlatform,
576594
);
577595

578596
expect(() => fileSystem.currentDirectory, throwsToolExit(message: 'Unable to read current working directory'));
579597
});
580598

581-
testWithoutContext('Rethrows os error $kSystemCannotFindFile', () {
599+
testWithoutContext('Rethrows os error $kSystemCodeCannotFindFile', () {
582600
final ErrorHandlingFileSystem fileSystem = ErrorHandlingFileSystem(
583601
delegate: MemoryFileSystem.test(opHandle: exceptionHandler.opHandle),
584602
platform: linuxPlatform,
@@ -588,11 +606,11 @@ void main() {
588606
exceptionHandler.addError(
589607
file,
590608
FileSystemOp.read,
591-
FileSystemException('', file.path, const OSError('', kSystemCannotFindFile)),
609+
FileSystemException('', file.path, const OSError('', kSystemCodeCannotFindFile)),
592610
);
593611

594612
// Error is not caught by other operations.
595-
expect(() => fileSystem.file('foo').readAsStringSync(), throwsFileSystemException(kSystemCannotFindFile));
613+
expect(() => fileSystem.file('foo').readAsStringSync(), throwsFileSystemException(kSystemCodeCannotFindFile));
596614
});
597615
});
598616

0 commit comments

Comments
 (0)