Skip to content

Commit b42700f

Browse files
srawlinsCommit Queue
authored andcommitted
DAS plugins: Print plugin pub-related crash information to terminal; stop analyzing
Change-Id: Id7f3d9957c52239210029b3b5c3e6f58d96a4d21 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/413000 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 58ff654 commit b42700f

File tree

13 files changed

+290
-10
lines changed

13 files changed

+290
-10
lines changed

pkg/analysis_server/doc/api.html

Lines changed: 24 additions & 1 deletion
Large diffs are not rendered by default.

pkg/analysis_server/lib/protocol/protocol_constants.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,8 @@ const String SERVER_NOTIFICATION_ERROR_MESSAGE = 'message';
325325
const String SERVER_NOTIFICATION_ERROR_STACK_TRACE = 'stackTrace';
326326
const String SERVER_NOTIFICATION_LOG = 'server.log';
327327
const String SERVER_NOTIFICATION_LOG_ENTRY = 'entry';
328+
const String SERVER_NOTIFICATION_PLUGIN_ERROR = 'server.pluginError';
329+
const String SERVER_NOTIFICATION_PLUGIN_ERROR_MESSAGE = 'message';
328330
const String SERVER_NOTIFICATION_STATUS = 'server.status';
329331
const String SERVER_NOTIFICATION_STATUS_ANALYSIS = 'analysis';
330332
const String SERVER_NOTIFICATION_STATUS_PUB = 'pub';

pkg/analysis_server/lib/protocol/protocol_generated.dart

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19053,6 +19053,87 @@ class ServerOpenUrlRequestResult implements ResponseResult {
1905319053
int get hashCode => 561630021;
1905419054
}
1905519055

19056+
/// server.pluginError params
19057+
///
19058+
/// {
19059+
/// "message": String
19060+
/// }
19061+
///
19062+
/// Clients may not extend, implement or mix-in this class.
19063+
class ServerPluginErrorParams implements HasToJson {
19064+
/// The error message indicating what kind of error was encountered.
19065+
String message;
19066+
19067+
ServerPluginErrorParams(this.message);
19068+
19069+
factory ServerPluginErrorParams.fromJson(
19070+
JsonDecoder jsonDecoder,
19071+
String jsonPath,
19072+
Object? json, {
19073+
required ClientUriConverter? clientUriConverter,
19074+
}) {
19075+
json ??= {};
19076+
if (json is Map) {
19077+
String message;
19078+
if (json.containsKey('message')) {
19079+
message = jsonDecoder.decodeString(
19080+
'$jsonPath.message',
19081+
json['message'],
19082+
);
19083+
} else {
19084+
throw jsonDecoder.mismatch(jsonPath, 'message');
19085+
}
19086+
return ServerPluginErrorParams(message);
19087+
} else {
19088+
throw jsonDecoder.mismatch(jsonPath, 'server.pluginError params', json);
19089+
}
19090+
}
19091+
19092+
factory ServerPluginErrorParams.fromNotification(
19093+
Notification notification, {
19094+
required ClientUriConverter? clientUriConverter,
19095+
}) {
19096+
return ServerPluginErrorParams.fromJson(
19097+
ResponseDecoder(null),
19098+
'params',
19099+
notification.params,
19100+
clientUriConverter: clientUriConverter,
19101+
);
19102+
}
19103+
19104+
@override
19105+
Map<String, Object> toJson({
19106+
required ClientUriConverter? clientUriConverter,
19107+
}) {
19108+
var result = <String, Object>{};
19109+
result['message'] = message;
19110+
return result;
19111+
}
19112+
19113+
Notification toNotification({
19114+
required ClientUriConverter? clientUriConverter,
19115+
}) {
19116+
return Notification(
19117+
'server.pluginError',
19118+
toJson(clientUriConverter: clientUriConverter),
19119+
);
19120+
}
19121+
19122+
@override
19123+
String toString() => json.encode(toJson(clientUriConverter: null));
19124+
19125+
@override
19126+
bool operator ==(other) {
19127+
if (other is ServerPluginErrorParams) {
19128+
return message == other.message;
19129+
}
19130+
return false;
19131+
}
19132+
19133+
@override
19134+
int get hashCode => message.hashCode;
19135+
}
19136+
1905619137
/// ServerService
1905719138
///
1905819139
/// enum {

pkg/analysis_server/lib/src/lsp/notification_manager.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ class LspNotificationManager extends AbstractNotificationManager {
8888
// on-demand.
8989
}
9090

91+
@override
92+
void sendPluginError(String message) {
93+
// TODO(srawlins): Implement.
94+
}
95+
9196
@override
9297
void sendPluginErrorNotification(Notification notification) {
9398
// TODO(dantup): implement sendPluginErrorNotification

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

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,20 @@ abstract class AbstractNotificationManager {
9595
Stream<bool> get pluginAnalysisStatusChanges =>
9696
_analysisStatusChangesController.stream;
9797

98+
/// Handles a plugin error.
99+
///
100+
/// This amounts to notifying the client, and setting the 'is analyzing'
101+
/// status to `false`.
102+
///
103+
/// The caller is responsible for logging the error with the instrumentation
104+
/// service.
105+
void handlePluginError(String message) {
106+
_analysisStatusChangesController.add(false /* isAnalyzing */);
107+
pluginStatusAnalyzing = false;
108+
109+
sendPluginError(message);
110+
}
111+
98112
/// Handle the given [notification] from the plugin with the given [pluginId].
99113
void handlePluginNotification(
100114
String pluginId,
@@ -260,6 +274,9 @@ abstract class AbstractNotificationManager {
260274
@visibleForOverriding
261275
void sendOutlines(String filePath, List<Outline> mergedOutlines);
262276

277+
@visibleForOverriding
278+
void sendPluginError(String message);
279+
263280
/// Sends plugin errors to the client.
264281
@visibleForOverriding
265282
void sendPluginErrorNotification(plugin.Notification notification);
@@ -363,11 +380,6 @@ abstract class AbstractNotificationManager {
363380
}
364381
var isAnalyzing = analysis.isAnalyzing;
365382
_analysisStatusChangesController.add(isAnalyzing);
366-
367-
// Only send status when it changes.
368-
if (pluginStatusAnalyzing == isAnalyzing) {
369-
return;
370-
}
371383
pluginStatusAnalyzing = isAnalyzing;
372384
}
373385
}
@@ -449,6 +461,15 @@ class NotificationManager extends AbstractNotificationManager {
449461
);
450462
}
451463

464+
@override
465+
void sendPluginError(String message) {
466+
_channel.sendNotification(
467+
server.ServerPluginErrorParams(
468+
message,
469+
).toNotification(clientUriConverter: uriConverter),
470+
);
471+
}
472+
452473
/// Sends plugin errors to the client.
453474
@override
454475
void sendPluginErrorNotification(plugin.Notification notification) {

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -702,13 +702,16 @@ class PluginManager {
702702
String? exceptionReason;
703703
if (result.exitCode != 0) {
704704
var buffer = StringBuffer();
705-
buffer.writeln('Failed to run pub $pubCommand');
706-
buffer.writeln(' pluginFolder = ${pluginFolder.path}');
705+
buffer.writeln(
706+
'An error occurred while setting up the analyzer plugin package at '
707+
"'${pluginFolder.path}'. The `dart pub $pubCommand` command failed:",
708+
);
707709
buffer.writeln(' exitCode = ${result.exitCode}');
708710
buffer.writeln(' stdout = ${result.stdout}');
709711
buffer.writeln(' stderr = ${result.stderr}');
710712
exceptionReason = buffer.toString();
711713
instrumentationService.logError(exceptionReason);
714+
notificationManager.handlePluginError(exceptionReason);
712715
}
713716
if (!packageConfigFile.exists) {
714717
exceptionReason ??= 'File "${packageConfigFile.path}" does not exist.';
@@ -930,8 +933,7 @@ class PluginSession {
930933
/// response to those requests.
931934
@visibleForTesting
932935
// ignore: library_private_types_in_public_api
933-
Map<String, _PendingRequest>
934-
pendingRequests = <String, _PendingRequest>{};
936+
Map<String, _PendingRequest> pendingRequests = <String, _PendingRequest>{};
935937

936938
/// A boolean indicating whether the plugin is compatible with the version of
937939
/// the plugin API being used by this server.

pkg/analysis_server/test/integration/support/integration_test_methods.dart

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,24 @@ abstract class IntegrationTest {
290290
/// Stream controller for [onServerError].
291291
final _onServerError = StreamController<ServerErrorParams>(sync: true);
292292

293+
/// Reports that an unexpected error has occurred while setting up an
294+
/// analyzer plugin, or during a plugin's execution.
295+
///
296+
/// It is not possible to subscribe to or unsubscribe from this notification.
297+
///
298+
/// Parameters
299+
///
300+
/// message: String
301+
///
302+
/// The error message indicating what kind of error was encountered.
303+
late final Stream<ServerPluginErrorParams> onServerPluginError =
304+
_onServerPluginError.stream.asBroadcastStream();
305+
306+
/// Stream controller for [onServerPluginError].
307+
final _onServerPluginError = StreamController<ServerPluginErrorParams>(
308+
sync: true,
309+
);
310+
293311
/// The stream of entries describing events happened in the server.
294312
///
295313
/// Parameters
@@ -3073,6 +3091,16 @@ abstract class IntegrationTest {
30733091
clientUriConverter: uriConverter,
30743092
),
30753093
);
3094+
case 'server.pluginError':
3095+
outOfTestExpect(params, isServerPluginErrorParams);
3096+
_onServerPluginError.add(
3097+
ServerPluginErrorParams.fromJson(
3098+
decoder,
3099+
'params',
3100+
params,
3101+
clientUriConverter: uriConverter,
3102+
),
3103+
);
30763104
case 'server.log':
30773105
outOfTestExpect(params, isServerLogParams);
30783106
_onServerLog.add(

pkg/analysis_server/test/integration/support/protocol_matchers.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3482,6 +3482,15 @@ final Matcher isServerOpenUrlRequestParams = LazyMatcher(
34823482
/// server.openUrlRequest result
34833483
final Matcher isServerOpenUrlRequestResult = isNull;
34843484

3485+
/// server.pluginError params
3486+
///
3487+
/// {
3488+
/// "message": String
3489+
/// }
3490+
final Matcher isServerPluginErrorParams = LazyMatcher(
3491+
() => MatchesJsonObject('server.pluginError params', {'message': isString}),
3492+
);
3493+
34853494
/// server.setClientCapabilities params
34863495
///
34873496
/// {

pkg/analysis_server/tool/spec/spec_input.html

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,26 @@ <h4>Options</h4>
581581
</field>
582582
</params>
583583
</notification>
584+
<notification event="pluginError">
585+
<p>
586+
Reports that an unexpected error has occurred while
587+
setting up an analyzer plugin, or during a plugin's
588+
execution.
589+
</p>
590+
<p>
591+
It is not possible to subscribe to or unsubscribe from this
592+
notification.
593+
</p>
594+
<params>
595+
<field name="message">
596+
<ref>String</ref>
597+
<p>
598+
The error message indicating what kind of error was
599+
encountered.
600+
</p>
601+
</field>
602+
</params>
603+
</notification>
584604
<notification event="log" experimental="true">
585605
<p>
586606
The stream of entries describing events happened in the server.

pkg/analysis_server_client/lib/handler/notification_handler.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ mixin NotificationHandler {
9797
case SERVER_NOTIFICATION_LOG:
9898
onServerLog(ServerLogParams.fromJson(decoder, 'params', params));
9999
break;
100+
case SERVER_NOTIFICATION_PLUGIN_ERROR:
101+
onServerPluginError(
102+
ServerPluginErrorParams.fromJson(decoder, 'params', params));
103+
break;
100104
case SERVER_NOTIFICATION_STATUS:
101105
onServerStatus(ServerStatusParams.fromJson(decoder, 'params', params));
102106
break;
@@ -264,6 +268,14 @@ mixin NotificationHandler {
264268
/// The stream of entries describing events happened in the server.
265269
void onServerLog(ServerLogParams params) {}
266270

271+
/// Reports that an unexpected error has occurred while
272+
/// setting up an analyzer plugin, or during a plugin's
273+
/// execution.
274+
///
275+
/// It is not possible to subscribe to or unsubscribe from this
276+
/// notification.
277+
void onServerPluginError(ServerPluginErrorParams params) {}
278+
267279
/// Reports the current status of the server. Parameters are
268280
/// omitted if there has been no change in the status
269281
/// represented by that parameter.

0 commit comments

Comments
 (0)