Add the hooks to capture analytics for requests and responses
In the process of testing this I found (and fixed) a couple of bugs
introduced in the recent conversion of the legacy code to handler
classes. I wanted to validate that I'd added hooks in all the right
places, so I wrote an AnalysisManager to keep track of request/response
pairs. It showed that some extra responses were being generated. I'm
still debating how best to add this to the normal tests to prevent
regressions, so you'll probably see it in another CL at some point.
Change-Id: Ib203c02ef27b6efb53a41a48696a3a39e61ed800
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245260
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Danny Tuppeny <danny@tuppeny.com>
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index 0bd5019..5974884 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -439,6 +439,8 @@
/// Handle a [request] that was read from the communication channel.
void handleRequest(Request request) {
+ analyticsManager.startedRequest(
+ request: request, startTime: DateTime.now());
performance.logRequestTiming(request.clientRequestTime);
// Because we don't `await` the execution of the handlers, we wrap the
// execution in order to have one central place to handle exceptions.
@@ -510,6 +512,7 @@
/// Send the given [response] to the client.
void sendResponse(Response response) {
channel.sendResponse(response);
+ analyticsManager.sentResponse(response: response);
cancellationTokens.remove(response.id);
}
diff --git a/pkg/analysis_server/lib/src/handler/legacy/analysis_set_priority_files.dart b/pkg/analysis_server/lib/src/handler/legacy/analysis_set_priority_files.dart
index c8d21c2..90fc2e5 100644
--- a/pkg/analysis_server/lib/src/handler/legacy/analysis_set_priority_files.dart
+++ b/pkg/analysis_server/lib/src/handler/legacy/analysis_set_priority_files.dart
@@ -23,6 +23,7 @@
for (var file in params.files) {
if (!server.isAbsoluteAndNormalized(file)) {
sendResponse(Response.invalidFilePathFormat(request, file));
+ return;
}
}
diff --git a/pkg/analysis_server/lib/src/handler/legacy/analysis_update_content.dart b/pkg/analysis_server/lib/src/handler/legacy/analysis_update_content.dart
index b2852185..61f5029 100644
--- a/pkg/analysis_server/lib/src/handler/legacy/analysis_update_content.dart
+++ b/pkg/analysis_server/lib/src/handler/legacy/analysis_update_content.dart
@@ -23,6 +23,7 @@
for (var file in params.files.keys) {
if (!server.isAbsoluteAndNormalized(file)) {
sendResponse(Response.invalidFilePathFormat(request, file));
+ return;
}
}
diff --git a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
index 2a54557..bbcd39b 100644
--- a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
+++ b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
@@ -343,6 +343,7 @@
/// Handle a [message] that was read from the communication channel.
void handleMessage(Message message) {
+ var startTime = DateTime.now();
performance.logRequestTiming(message.clientRequestTime);
runZonedGuarded(() async {
try {
@@ -367,6 +368,8 @@
);
if (message is RequestMessage) {
+ analyticsManager.startedRequestMessage(
+ request: message, startTime: startTime);
await _handleRequestMessage(message, messageInfo);
} else if (message is NotificationMessage) {
await _handleNotificationMessage(message, messageInfo);
@@ -567,7 +570,7 @@
void sendErrorResponse(Message message, ResponseError error) {
if (message is RequestMessage) {
- channel.sendResponse(ResponseMessage(
+ sendResponse(ResponseMessage(
id: message.id, error: error, jsonrpc: jsonRpcVersion));
} else if (message is ResponseMessage) {
// For bad response messages where we can't respond with an error, send it
@@ -618,6 +621,7 @@
/// Send the given [response] to the client.
void sendResponse(ResponseMessage response) {
channel.sendResponse(response);
+ analyticsManager.sentResponseMessage(response: response);
}
@override
@@ -800,7 +804,7 @@
if (result.isError) {
sendErrorResponse(message, result.error);
} else {
- channel.sendResponse(ResponseMessage(
+ sendResponse(ResponseMessage(
id: message.id,
result: result.result,
jsonrpc: jsonRpcVersion,
diff --git a/pkg/analysis_server/test/analysis/set_priority_files_test.dart b/pkg/analysis_server/test/analysis/set_priority_files_test.dart
index f64c656..6817bb5 100644
--- a/pkg/analysis_server/test/analysis/set_priority_files_test.dart
+++ b/pkg/analysis_server/test/analysis/set_priority_files_test.dart
@@ -28,14 +28,14 @@
Future<void> test_fileDoesNotExist() async {
var file = getFile('$testPackageLibPath/doesNotExist.dart');
var response = await _setPriorityFile(file);
- expect(response, isResponseSuccess('0'));
+ expect(response, isResponseSuccess('1'));
}
Future<void> test_fileInAnalysisRoot() async {
addTestFile('');
// set priority files
var response = await _setPriorityFile(testFile);
- expect(response, isResponseSuccess('0'));
+ expect(response, isResponseSuccess('1'));
// verify
_verifyPriorityFiles(testFile);
}
@@ -55,7 +55,7 @@
.getChildAssumingFolder('convert')
.getChildAssumingFile('convert.dart');
var response = await _setPriorityFile(file);
- expect(response, isResponseSuccess('0'));
+ expect(response, isResponseSuccess('1'));
// verify
_verifyPriorityFiles(file);
}
@@ -116,11 +116,11 @@
var response = await handleRequest(
AnalysisSetPriorityFilesParams([
'test.dart',
- ]).toRequest('0'),
+ ]).toRequest('1'),
);
assertResponseFailure(
response,
- requestId: '0',
+ requestId: '1',
errorCode: RequestErrorCode.INVALID_FILE_PATH_FORMAT,
);
}
@@ -128,10 +128,10 @@
Future<void> test_invalidFilePathFormat_notNormalized() async {
var response = await handleRequest(AnalysisSetPriorityFilesParams([
convertPath('/foo/../bar/test.dart'),
- ]).toRequest('0'));
+ ]).toRequest('1'));
assertResponseFailure(
response,
- requestId: '0',
+ requestId: '1',
errorCode: RequestErrorCode.INVALID_FILE_PATH_FORMAT,
);
}
@@ -140,7 +140,7 @@
addTestFile('');
// set priority files
var response = await _setPriorityFile(testFile);
- expect(response, isResponseSuccess('0'));
+ expect(response, isResponseSuccess('1'));
// verify
var params = pluginManager.analysisSetPriorityFilesParams!;
expect(params.files, [testFile.path]);
@@ -150,7 +150,7 @@
return await handleSuccessfulRequest(
AnalysisSetPriorityFilesParams(<String>[
file.path,
- ]).toRequest('0'),
+ ]).toRequest('1'),
);
}
