[flutter_tools] fix response format of flutterVersion, flutterMemoryInfo #54786
Conversation
fluttergithubbot
added
the
tool
There was a problem hiding this comment.
lgtm
@bkonyi, it would be worth thinking about ways we could improve either the VM service protocol or the VM service lib so that service protocol extensions could just return their results (instead of carefully packing their results into well-formed data structures).
| return versionJson; | ||
| return <String, dynamic>{ | ||
| 'result': <String, Object>{ | ||
| 'type': 'Success', |
There was a problem hiding this comment.
This looks correct (comparing to the compileExpression implementation).
cc @bkonyi - it looks like each client registering a vm service extension needs to be very careful about the response it creates
|
Might be worth seeing if there's something we can do to abstract this out and make it easier to get it right -- e.g. having a general API for sending responses so that only one place needs to create this JSON blob. (As a separate PR, obviously, not trying to hold this up.) |
|
Yeah, Ideally the callback here would accept a vm_service.Response type. Then we could construct a well-typed object like vm_service.Success and avoid trying to match up the JSON structure |
Description
When the response structure of this JSON is incorrect, it causes the client to crash. Fix the format and write an integration test that confirms registration and calling works correctly.
Fixes #54755