languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-16 00:29:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

analyzer: Do not overwrite an original exception when a plugin crashes

Browse source 
Maybe related to https://github.com/dart-lang/sdk/issues/38629. These tests have been skipped for so long, enabling them took some work, to migrate them from '.packages' files to package config files.

Some other tidying in the test file:

* inline `byteStorePath`, only used once.
* simplify `_packagesFileContent` and `_getPackagesFileContent`
  into a static getter.
* simplify `_defaultPluginContent` into a const String, so it can
  be used as a function parameter default value

The diff is way bigger than the functional changes, because we sort
elements.


This reverts commit aa6b6470e3.

Change-Id: I9dc533710255534ec27454712a1a64facf5dd12d
Cq-Include-Trybots: luci.dart.try:analyzer-win-release-try,flutter-analyze-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345367
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This commit is contained in:
Sam Rawlins 2024-01-10 17:47:52 +00:00 committed by Commit Queue
parent ebf1456c7b
commit 6b6f333e00
2 changed files with 113 additions and 112 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
pkg/analysis_server/lib/src/plugin/plugin_manager.dart
View file

@ -196,7 +196,9 @@ abstract class PluginInfo {
}
void reportException(CaughtException exception) {
_exception = exception;
// If a previous exception has been reported, do not replace it here; the
//first should have more "root cause" information.
_exception ??= exception;
instrumentationService.logPluginException(
data, exception.exception, exception.stackTrace);
}
@ -972,7 +974,7 @@ class PluginSession {
onDone: handleOnDone, onError: handleOnError) as dynamic);
if (channel == null) {
// If there is an error when starting the isolate, the channel will invoke
// handleOnDone, which will cause `channel` to be set to `null`.
// `handleOnDone`, which will cause `channel` to be set to `null`.
info.reportException(CaughtException(
PluginException('Unrecorded error while starting the plugin.'),
StackTrace.current));

219
pkg/analysis_server/test/src/plugin/plugin_manager_test.dart
View file

@ -187,13 +187,12 @@ class DiscoveredPluginInfoTest with ResourceProviderMixin, _ContextRoot {
@reflectiveTest
class PluginManagerFromDiskTest extends PluginTestSupport {
String byteStorePath = '/byteStore';
late PluginManager manager;
@override
void setUp() {
super.setUp();
manager = PluginManager(resourceProvider, byteStorePath, '',
manager = PluginManager(resourceProvider, '/byteStore', '',
notificationManager, InstrumentationService.NULL_SERVICE);
}
@ -284,9 +283,6 @@ class PluginManagerFromDiskTest extends PluginTestSupport {
pkg1Dir.deleteSync(recursive: true);
}
@SkippedTest(
reason: 'flaky timeouts',
issue: 'https://github.com/dart-lang/sdk/issues/38629')
Future<void> test_broadcastRequest_noCurrentSession() async {
var pkg1Dir = io.Directory.systemTemp.createTempSync('pkg1');
var pkgPath = pkg1Dir.resolveSymbolicLinksSync();
@ -298,11 +294,26 @@ class PluginManagerFromDiskTest extends PluginTestSupport {
await manager.addPluginToContextRoot(contextRoot, plugin1Path);
var responses = manager.broadcastRequest(
CompletionGetSuggestionsParams('/pkg1/lib/pkg1.dart', 100),
contextRoot: contextRoot);
CompletionGetSuggestionsParams('/pkg1/lib/pkg1.dart', 100),
contextRoot: contextRoot,
);
expect(responses, hasLength(0));
await manager.stopAll();
var exception = manager.plugins.first.exception;
expect(exception, isNotNull);
var innerException = exception!.exception;
expect(
innerException,
isA<PluginException>().having(
(e) => e.message,
'message',
allOf(
contains('Unable to spawn isolate'),
contains('(invalid content here)'),
),
),
);
});
pkg1Dir.deleteSync(recursive: true);
}
@ -441,7 +452,12 @@ class PluginManagerFromDiskTest extends PluginTestSupport {
}
ContextRootImpl _newContextRoot(String root) {
throw UnimplementedError();
root = resourceProvider.convertPath(root);
return ContextRootImpl(
resourceProvider,
resourceProvider.getFolder(root),
BasicWorkspace.find(resourceProvider, Packages.empty, root),
);
}
}
@ -689,92 +705,12 @@ class PluginSessionTest with ResourceProviderMixin {
}
}
/// A class designed to be used as a superclass for test classes that define
/// tests that require plugins to be created on disk.
/// A superclass for test classes that define tests that require plugins to be
/// created on disk.
abstract class PluginTestSupport {
late PhysicalResourceProvider resourceProvider;
late TestNotificationManager notificationManager;
/// The content to be used for the '.packages' file, or `null` if the content
/// has not yet been computed.
String? _packagesFileContent;
void setUp() {
resourceProvider = PhysicalResourceProvider.INSTANCE;
notificationManager = TestNotificationManager();
}
/// Create a directory structure representing a plugin on disk, run the given
/// [test] function, and then remove the directory. The directory will have
/// the following structure:
/// ```
/// pluginDirectory
/// .packages
/// bin
/// plugin.dart
/// ```
/// The name of the plugin directory will be the [pluginName], if one is
/// provided (in order to allow more than one plugin to be created by a single
/// test). The 'plugin.dart' file will contain the given [content], or default
/// content that implements a minimal plugin if the contents are not given.
/// The [test] function will be passed the path of the directory that was
/// created.
Future<void> withPlugin(
{String? content,
String? pluginName,
required Future<void> Function(String) test}) async {
var tempDirectory =
io.Directory.systemTemp.createTempSync(pluginName ?? 'test_plugin');
try {
var pluginPath = tempDirectory.resolveSymbolicLinksSync();
//
// Create a .packages file.
//
var packagesFile = io.File(path.join(pluginPath, '.packages'));
packagesFile.writeAsStringSync(_getPackagesFileContent());
//
// Create the 'bin' directory.
//
var binPath = path.join(pluginPath, 'bin');
io.Directory(binPath).createSync();
//
// Create the 'plugin.dart' file.
//
var pluginFile = io.File(path.join(binPath, 'plugin.dart'));
pluginFile.writeAsStringSync(content ?? _defaultPluginContent());
//
// Run the actual test code.
//
await test(pluginPath);
} finally {
tempDirectory.deleteSync(recursive: true);
}
}
/// Convert the [sdkPackageMap] into a plugin-specific map by applying the
/// given relative path [delta] to each line.
String _convertPackageMap(String sdkDirPath, List<String> sdkPackageMap) {
var buffer = StringBuffer();
for (var line in sdkPackageMap) {
if (!line.startsWith('#')) {
var index = line.indexOf(':');
var packageName = line.substring(0, index + 1);
var relativePath = line.substring(index + 1);
var absolutePath = path.join(sdkDirPath, relativePath);
// Convert to file:/// URI since that's how absolute paths in
// .packages must be for windows
absolutePath = Uri.file(absolutePath).toString();
buffer.write(packageName);
buffer.writeln(absolutePath);
}
}
return buffer.toString();
}
/// The default content of the plugin. This is a minimal plugin that will only
/// respond correctly to version checks and to shutdown requests.
String _defaultPluginContent() {
return r'''
static const _defaultPluginContent = r'''
import 'dart:async';
import 'dart:isolate';
import 'package:analyzer/file_system/file_system.dart';
@ -814,30 +750,79 @@ class MinimalPlugin extends ServerPlugin {
bool isCompatibleWith(Version serverVersion) => true;
}
''';
/// The path to the package config file in the root of the Dart SDK.
static String get _sdkPackageConfigPath {
var filePath = io.Platform.script.toFilePath();
// Walk up the directory structure from this script file to the
// 'analysis_server' root directory.
while (
filePath.isNotEmpty && path.basename(filePath) != 'analysis_server') {
filePath = path.dirname(filePath);
}
filePath = path.dirname(path.dirname(filePath));
return path.join(filePath, '.dart_tool', 'package_config.json');
}
/// Return the content to be used for the '.packages' file.
String _getPackagesFileContent() {
var packagesFileContent = _packagesFileContent;
if (packagesFileContent == null) {
var sdkPackagesFile = io.File(_sdkPackagesPath());
var sdkPackageMap = sdkPackagesFile.readAsLinesSync();
packagesFileContent = _packagesFileContent =
_convertPackageMap(path.dirname(sdkPackagesFile.path), sdkPackageMap);
}
return packagesFileContent;
late PhysicalResourceProvider resourceProvider;
late TestNotificationManager notificationManager;
void setUp() {
resourceProvider = PhysicalResourceProvider.INSTANCE;
notificationManager = TestNotificationManager();
}
/// Return the path to the '.packages' file in the root of the SDK checkout.
String _sdkPackagesPath() {
var packagesPath = io.Platform.script.toFilePath();
while (packagesPath.isNotEmpty &&
path.basename(packagesPath) != 'analysis_server') {
packagesPath = path.dirname(packagesPath);
/// Creates a directory structure representing a plugin on disk, runs the
/// given [test] function, and then removes the directory.
///
/// The directory will have the following structure:
/// ```
/// <pluginDirectory>
/// .dart_tool/
/// package_config.dart
/// bin/
/// plugin.dart
/// ```
/// The name of the plugin directory will be the [pluginName], if one is
/// provided (in order to allow more than one plugin to be created by a single
/// test). The 'plugin.dart' file will contain the given [content], or default
/// content that implements a minimal plugin if the contents are not given.
/// The [test] function will be passed the path of the directory that was
/// created.
Future<void> withPlugin({
String content = _defaultPluginContent,
String pluginName = 'test_plugin',
required Future<void> Function(String) test,
}) async {
var tempDirectory = io.Directory.systemTemp.createTempSync(pluginName);
try {
var pluginPath = tempDirectory.resolveSymbolicLinksSync();
// Create a package config file.
var pluginDartToolPath = path.join(pluginPath, '.dart_tool');
io.Directory(pluginDartToolPath).createSync();
var packageConfigFile = io.File(
path.join(pluginDartToolPath, 'package_config.json'),
);
packageConfigFile
.writeAsStringSync(io.File(_sdkPackageConfigPath).readAsStringSync());
//
// Create the 'bin' directory.
//
var binPath = path.join(pluginPath, 'bin');
io.Directory(binPath).createSync();
//
// Create the 'plugin.dart' file.
//
var pluginFile = io.File(path.join(binPath, 'plugin.dart'));
pluginFile.writeAsStringSync(content);
//
// Run the actual test code.
//
await test(pluginPath);
} finally {
tempDirectory.deleteSync(recursive: true);
}
packagesPath = path.dirname(packagesPath);
packagesPath = path.dirname(packagesPath);
return path.join(packagesPath, '.packages');
}
}
@ -910,3 +895,17 @@ mixin _ContextRoot on ResourceProviderMixin {
);
}
}
extension on PhysicalResourceProvider {
/// Converts the given posix [filePath] to conform to this provider's path
/// context.
String convertPath(String filePath) {
if (pathContext.style == path.windows.style) {
if (filePath.startsWith(path.posix.separator)) {
filePath = r'C:' + filePath;
}
return filePath.replaceAll(path.posix.separator, path.windows.separator);
}
return filePath;
}
}