[analysis_server] Don't endlessly rebuild analysis contexts on PathNotFound errors

Fixes https://github.com/Dart-Code/Dart-Code/issues/4280.

Change-Id: I8f36672dfd5fcca2e4be87408d385219b0a81d9c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284660
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Danny Tuppeny 2023-02-28 20:27:43 +00:00 committed by Commit Queue
parent b4fd9ac97f
commit e231f12420
5 changed files with 108 additions and 3 deletions

View file

@ -769,6 +769,16 @@ class ContextManagerImpl implements ContextManager {
/// On windows, the directory watcher may overflow, and we must recover.
void _handleWatchInterruption(dynamic error, StackTrace stackTrace) {
// If the watcher failed because the directory does not exist, rebuilding
// the contexts will result in infinite looping because it will just
// re-occur.
// https://github.com/Dart-Code/Dart-Code/issues/4280
if (error is PathNotFoundException) {
_instrumentationService.logError('Watcher error; not refreshing contexts '
'because PathNotFound.\n$error\n$stackTrace');
return;
}
// We've handled the error, so we only have to log it.
_instrumentationService
.logError('Watcher error; refreshing contexts.\n$error\n$stackTrace');

View file

@ -20,6 +20,62 @@ void main() {
@reflectiveTest
class ServerTest extends AbstractLspAnalysisServerTest {
List<String> get currentContextPaths => server.contextManager.analysisContexts
.map((context) => context.contextRoot.root.path)
.toList();
/// Ensure an analysis root that doesn't exist does not cause an infinite
/// rebuild loop.
/// https://github.com/Dart-Code/Dart-Code/issues/4280
Future<void> test_analysisRoot_doesNotExist() async {
const notExistingPath = '/does/not/exist';
resourceProvider.emitPathNotFoundExceptionsForPaths.add(notExistingPath);
await initialize(workspaceFolders: [Uri.file(notExistingPath)]);
// Wait a short period and ensure there was exactly one context build.
await pumpEventQueue(times: 10000);
expect(server.contextBuilds, 1);
// And that the roots are as expected.
expect(
currentContextPaths,
unorderedEquals([
// TODO(dantup): It may be a bug that ContextLocator is producing
// contexts at the root for missing folders.
convertPath('/'), // the first existing ancestor of the requested folder
]),
);
}
Future<void> test_analysisRoot_existsAndDoesNotExist() async {
const notExistingPath = '/does/not/exist';
resourceProvider.emitPathNotFoundExceptionsForPaths.add(notExistingPath);
// Track diagnostics for the file to ensure we're analyzing the existing
// root.
final diagnosticsFuture = waitForDiagnostics(mainFileUri);
newFile(mainFilePath, 'NotAClass a;');
await initialize(
workspaceFolders: [projectFolderUri, Uri.file(notExistingPath)],
);
// Wait a short period and ensure there was exactly one context build.
await pumpEventQueue(times: 10000);
expect(server.contextBuilds, 1);
// And that the roots are as expected.
expect(
currentContextPaths,
unorderedEquals([
projectFolderPath,
convertPath('/'), // the first existing ancestor of the requested folder
]),
);
final diagnostics = await diagnosticsFuture;
expect(diagnostics, hasLength(1));
expect(diagnostics!.single.code, 'undefined_class');
}
Future<void> test_capturesLatency_afterStartup() async {
await initialize(includeClientRequestTime: true);
await openFile(mainFileUri, '');

View file

@ -149,6 +149,15 @@ abstract class Folder implements Resource {
ResourceWatcher watch();
}
/// Exception thrown when a file operation fails because a file or directory
/// does not exist.
class PathNotFoundException extends FileSystemException {
PathNotFoundException(super.path, super.message);
@override
String toString() => 'PathNotFoundException(path=$path; message=$message)';
}
/// The abstract class [Resource] is an abstraction of file or folder.
abstract class Resource {
/// Return `true` if this resource exists.

View file

@ -33,6 +33,11 @@ class MemoryResourceProvider implements ResourceProvider {
@visibleForTesting
final Duration? delayWatcherInitialization;
/// Paths that should have `PathNotFoundException`s emitted on their watch
/// streams.
@visibleForTesting
final Set<String> emitPathNotFoundExceptionsForPaths = {};
MemoryResourceProvider({
pathos.Context? context,
this.delayWatcherInitialization,
@ -625,6 +630,10 @@ abstract class _MemoryResource implements Resource {
}
});
ready.complete();
if (provider.emitPathNotFoundExceptionsForPaths.contains(path)) {
streamController.addError(PathNotFoundException(
path, 'Simulated PathNotFoundException from _MemoryResource'));
}
}
final delayWatcherInitialization = provider.delayWatcherInitialization;

View file

@ -2,6 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:async';
import 'dart:io' as io;
import 'dart:typed_data';
@ -183,7 +184,7 @@ class _PhysicalFile extends _PhysicalResource implements File {
@override
ResourceWatcher watch() {
final watcher = FileWatcher(_entry.path);
return ResourceWatcher(watcher.events, watcher.ready);
return ResourceWatcher(_wrapWatcherStream(watcher.events), watcher.ready);
}
@override
@ -319,7 +320,7 @@ class _PhysicalFolder extends _PhysicalResource implements Folder {
// Don't suppress "Directory watcher closed," so the outer
// listener can see the interruption & act on it.
!error.message.startsWith("Directory watcher closed unexpectedly"));
return ResourceWatcher(events, watcher.ready);
return ResourceWatcher(_wrapWatcherStream(events), watcher.ready);
}
}
@ -408,6 +409,26 @@ abstract class _PhysicalResource implements Resource {
}
FileSystemException _wrapException(io.FileSystemException e) {
return FileSystemException(e.path ?? path, e.message);
if (e is io.PathNotFoundException) {
return PathNotFoundException(e.path ?? path, e.message);
} else {
return FileSystemException(e.path ?? path, e.message);
}
}
/// Wraps a `Stream<WatchEvent>` to map all known errors through
/// [_wrapException] into server types.
Stream<WatchEvent> _wrapWatcherStream(Stream<WatchEvent> original) {
/// Helper to map thrown `FileSystemException`s to servers abstraction.
Object mapException(Object e) {
return e is io.FileSystemException ? _wrapException(e) : e;
}
final mappedEventsController = StreamController<WatchEvent>();
original.listen(
mappedEventsController.add,
onError: (Object e) => mappedEventsController.addError(mapException(e)),
);
return mappedEventsController.stream;
}
}