Skip to content

Commit

Permalink
[ VM / Service ] Handle case where a JSON RPC notification results in…
Browse files Browse the repository at this point in the history
… an error

JSON RPC notifications never receive a response by design. In the VM
service, if an error needs to be returned for a request and the request
was a notification (i.e., has no ID associated with it), null is
returned. The Dart side of the service tries to create a Response object
with the null value, resulting in an exception being thrown. This change
will swallow the errors in situations where a notification RPC is
invalid.

Change-Id: If7d889150bfec51b933474f7a66fffb25e82daed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109200
Reviewed-by: Siva Annamalai <asiva@google.com>
  • Loading branch information
bkonyi committed Jul 16, 2019
1 parent 2dbd6dc commit 664c61d
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
7 changes: 6 additions & 1 deletion sdk/lib/vmservice/message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,12 @@ class Message {
}

void _setResponseFromPort(dynamic response) {
_completer.complete(new Response.from(response));
if (response == null) {
// We should only have a null response for Notifications.
assert(type == MessageType.Notification);
return null;
}
_completer.complete(Response.from(response));
}

void setResponse(String response) {
Expand Down
8 changes: 7 additions & 1 deletion sdk/lib/vmservice/vmservice.dart
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,13 @@ class VMService extends MessageRouter {
}

Future<Response> routeRequest(VMService _, Message message) async {
return new Response.from(await _routeRequestImpl(message));
final response = await _routeRequestImpl(message);
if (response == null) {
// We should only have a null response for Notifications.
assert(message.type == MessageType.Notification);
return null;
}
return new Response.from(response);
}

Future _routeRequestImpl(Message message) async {
Expand Down

0 comments on commit 664c61d

Please sign in to comment.