Skip to content

Commit 3f4a341

Browse files
srawlinsCommit Queue
authored andcommitted
DAS plugins: Log an isolate-spawning exception to terminal
Change-Id: Iafe061914a34d883b2683b5edae0ee73f75325c0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/413320 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent fa3461b commit 3f4a341

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

pkg/analysis_server/lib/src/plugin/plugin_manager.dart

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,13 +207,20 @@ abstract class PluginInfo {
207207

208208
void reportException(CaughtException exception) {
209209
// If a previous exception has been reported, do not replace it here; the
210-
//first should have more "root cause" information.
210+
// first should have more "root cause" information.
211211
_exception ??= exception;
212212
instrumentationService.logPluginException(
213213
data,
214214
exception.exception,
215215
exception.stackTrace,
216216
);
217+
var message =
218+
'An error occurred while executing an analyzer plugin: '
219+
// Sometimes the message is the primary information; sometimes the
220+
// exception is.
221+
'${exception.message ?? exception.exception}\n'
222+
'${exception.stackTrace}';
223+
notificationManager.handlePluginError(message);
217224
}
218225

219226
/// If the plugin is currently running, send a request based on the given
@@ -989,11 +996,19 @@ class PluginSession {
989996

990997
/// Handle the fact that an unhandled error has occurred in the plugin.
991998
void handleOnError(dynamic error) {
992-
var errorPair = (error as List).cast<String>();
993-
var stackTrace = StackTrace.fromString(errorPair[1]);
994-
info.reportException(
995-
CaughtException(PluginException(errorPair[0]), stackTrace),
996-
);
999+
if (error case [String message, String stackTraceString]) {
1000+
var stackTrace = StackTrace.fromString(stackTraceString);
1001+
var exception = PluginException(message);
1002+
info.reportException(
1003+
CaughtException.withMessage(message, exception, stackTrace),
1004+
);
1005+
} else {
1006+
throw ArgumentError.value(
1007+
error,
1008+
'error',
1009+
'expected to be a two-element List of Strings.',
1010+
);
1011+
}
9971012
}
9981013

9991014
/// Handle a [response] from the plugin by completing the future that was

pkg/analysis_server/test/src/plugin/plugin_manager_test.dart

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,10 +739,12 @@ class PluginSessionTest with ResourceProviderMixin {
739739
expect(session.pluginStoppedCompleter.isCompleted, isTrue);
740740
}
741741

742-
@failingTest
743742
void test_handleOnError() {
744743
session.handleOnError(<String>['message', 'trace']);
745-
fail('The method handleOnError is not implemented');
744+
expect(
745+
notificationManager.pluginErrors.first,
746+
'An error occurred while executing an analyzer plugin: message\ntrace',
747+
);
746748
}
747749

748750
Future<void> test_handleResponse() async {
@@ -781,6 +783,13 @@ class PluginSessionTest with ResourceProviderMixin {
781783
await session.start(path.join(pluginPath, 'byteStore'), sdkPath),
782784
isFalse,
783785
);
786+
expect(
787+
notificationManager.pluginErrors.first,
788+
startsWith(
789+
'An error occurred while executing an analyzer plugin: Plugin is not '
790+
'compatible.',
791+
),
792+
);
784793
}
785794

786795
Future<void> test_start_running() async {
@@ -933,6 +942,13 @@ class TestNotificationManager implements AbstractNotificationManager {
933942
Map<String, Map<String, List<AnalysisError>>> recordedErrors =
934943
<String, Map<String, List<AnalysisError>>>{};
935944

945+
List<String> pluginErrors = [];
946+
947+
@override
948+
void handlePluginError(String message) {
949+
pluginErrors.add(message);
950+
}
951+
936952
@override
937953
void handlePluginNotification(String pluginId, Notification notification) {
938954
notifications.add(notification);

0 commit comments

Comments
 (0)