[flutter_tool] Don't crash when failing to delete downloaded artifacts (#44933)

* [flutter_tool] Don't crash when failing to delete downloaded artifacts

* Add space

* Add more spaces.

packages/flutter_tools/lib/src/cache.dart

@@ -432,7 +432,8 @@ abstract class CachedArtifact extends ArtifactSet {
   /// can delete them after completion. We don't delete them right after
   /// extraction in case [update] is interrupted, so we can restart without
   /// starting from scratch.
-  final List<File> _downloadedFiles = <File>[];
+  @visibleForTesting
+  final List<File> downloadedFiles = <File>[];
 
   @override
   bool isUpToDate() {
@@ -465,8 +466,13 @@ abstract class CachedArtifact extends ArtifactSet {
 
   /// Clear any zip/gzip files downloaded.
   void _removeDownloadedFiles() {
-    for (File f in _downloadedFiles) {
-      f.deleteSync();
+    for (File f in downloadedFiles) {
+      try {
+        f.deleteSync();
+      } on FileSystemException catch (e) {
+        printError('Failed to delete "${f.path}". Please delete manually. $e');
+        continue;
+      }
       for (Directory d = f.parent; d.absolute.path != cache.getDownloadDir().absolute.path; d = d.parent) {
         if (d.listSync().isEmpty) {
           d.deleteSync();
@@ -532,10 +538,10 @@ abstract class CachedArtifact extends ArtifactSet {
   }
 
   /// Create a temporary file and invoke [onTemporaryFile] with the file as
-  /// argument, then add the temporary file to the [_downloadedFiles].
+  /// argument, then add the temporary file to the [downloadedFiles].
   Future<void> _withDownloadFile(String name, Future<void> onTemporaryFile(File file)) async {
     final File tempFile = fs.file(fs.path.join(cache.getDownloadDir().path, name));
-    _downloadedFiles.add(tempFile);
+    downloadedFiles.add(tempFile);
     await onTemporaryFile(tempFile);
   }
 }

packages/flutter_tools/test/general.shard/cache_test.dart

@@ -93,6 +93,27 @@ void main() {
       memoryFileSystem = MemoryFileSystem();
     });
 
+    testUsingContext('Continues on failed delete', () async {
+      final Directory artifactDir = fs.systemTempDirectory.createTempSync('flutter_cache_test_artifact.');
+      final Directory downloadDir = fs.systemTempDirectory.createTempSync('flutter_cache_test_download.');
+      when(mockCache.getArtifactDirectory(any)).thenReturn(artifactDir);
+      when(mockCache.getDownloadDir()).thenReturn(downloadDir);
+      final File mockFile = MockFile();
+      when(mockFile.deleteSync()).thenAnswer((_) {
+        throw const FileSystemException('delete failed');
+      });
+      final FakeDownloadedArtifact artifact = FakeDownloadedArtifact(
+        mockFile,
+        mockCache,
+      );
+      await artifact.update();
+      expect(testLogger.errorText, contains('delete failed'));
+    }, overrides: <Type, Generator>{
+      Cache: () => mockCache,
+      FileSystem: () => memoryFileSystem,
+      ProcessManager: () => FakeProcessManager.any(),
+    });
+
     testUsingContext('Gradle wrapper should not be up to date, if some cached artifact is not available', () {
       final GradleWrapper gradleWrapper = GradleWrapper(mockCache);
       final Directory directory = fs.directory('/Applications/flutter/bin/cache');
@@ -101,7 +122,7 @@ void main() {
       when(mockCache.getCacheDir(fs.path.join('artifacts', 'gradle_wrapper'))).thenReturn(fs.directory(fs.path.join(directory.path, 'artifacts', 'gradle_wrapper')));
       expect(gradleWrapper.isUpToDateInner(), false);
     }, overrides: <Type, Generator>{
-      Cache: ()=> mockCache,
+      Cache: () => mockCache,
       FileSystem: () => memoryFileSystem,
       ProcessManager: () => FakeProcessManager.any(),
     });
@@ -117,7 +138,7 @@ void main() {
       when(mockCache.getCacheDir(fs.path.join('artifacts', 'gradle_wrapper'))).thenReturn(fs.directory(fs.path.join(directory.path, 'artifacts', 'gradle_wrapper')));
       expect(gradleWrapper.isUpToDateInner(), true);
     }, overrides: <Type, Generator>{
-      Cache: ()=> mockCache,
+      Cache: () => mockCache,
       FileSystem: () => memoryFileSystem,
       ProcessManager: () => FakeProcessManager.any(),
     });
@@ -174,7 +195,7 @@ void main() {
         '/path/to/alpha:/path/to/beta:/path/to/gamma:/path/to/delta:/path/to/epsilon',
       );
     }, overrides: <Type, Generator>{
-      Cache: ()=> mockCache,
+      Cache: () => mockCache,
     });
     testUsingContext('failed storage.googleapis.com download shows China warning', () async {
       final CachedArtifact artifact1 = MockCachedArtifact();
@@ -272,7 +293,7 @@ void main() {
       expect(dir.path, artifactDir.childDirectory('bin_dir').path);
       verify(mockOperatingSystemUtils.chmod(argThat(hasPath(dir.path)), 'a+r,a+x'));
     }, overrides: <Type, Generator>{
-      Cache: ()=> mockCache,
+      Cache: () => mockCache,
       FileSystem: () => memoryFileSystem,
       ProcessManager: () => FakeProcessManager.any(),
       HttpClientFactory: () => () => fakeHttpClient,
@@ -324,7 +345,7 @@ void main() {
 
       expect(mavenArtifacts.isUpToDate(), isFalse);
     }, overrides: <Type, Generator>{
-      Cache: ()=> mockCache,
+      Cache: () => mockCache,
       FileSystem: () => memoryFileSystem,
       ProcessManager: () => processManager,
     });
@@ -435,6 +456,21 @@ class FakeCachedArtifact extends EngineCachedArtifact {
   List<String> getPackageDirs() => packageDirs;
 }
 
+class FakeDownloadedArtifact extends CachedArtifact {
+  FakeDownloadedArtifact(this.downloadedFile, Cache cache) : super(
+    'fake',
+    cache,
+    DevelopmentArtifact.universal,
+  );
+
+  final File downloadedFile;
+
+  @override
+  Future<void> updateInner() async {
+    downloadedFiles.add(downloadedFile);
+  }
+}
+
 class MockProcessManager extends Mock implements ProcessManager {}
 class MockFileSystem extends Mock implements FileSystem {}
 class MockFile extends Mock implements File {}