[analysis_server] Improve error reported by handleExpectedRequest when the expected request never arrives
The `handleExpectedRequest` test method sends a request to the server and expects a request back (for example executing a command that will trigger server-to-client applyEdit). There is a timeout (since this request may never come), but in the case where the timeout is caused by the outbound request failing, we should report that failure instead (since it is certainly the cause of the timeout).
This changes the code to capture the error from the outbound request, and _if_ the expected request never comes, throws that in preference to the timeout.
Before:
```
00:05 +0 -1: ApplyAllFixesInWorkspace | test_partFile_issue59572 [E]
TimeoutException after 0:00:05.000000: Future not completed
test\lsp\server_abstract.dart 1143:15 LspAnalysisServerTestMixin.expectRequest.<fn>
dart:async/zone.dart 1517:47 _rootRun
dart:async/zone.dart 1422:19 _CustomZone.run
dart:async/future_impl.dart 1036:34 Future.timeout.<fn>
===== asynchronous gap ===========================
test\lsp\server_abstract.dart 1139:29 LspAnalysisServerTestMixin.expectRequest
===== asynchronous gap ===========================
test\lsp\server_abstract.dart 1190:27 LspAnalysisServerTestMixin.handleExpectedRequest
===== asynchronous gap ===========================
test\lsp\server_abstract.dart 155:27 AbstractLspAnalysisServerTest.executeForEdits
```
After:
```
00:05 +0 -1: ApplyAllFixesInWorkspace | test_partFile_issue59572 [E]
{
"code": -32006,
"message": "dart.edit.fixAllInWorkspace requires a single Map argument"
}
test\lsp\server_abstract.dart 1211:7 LspAnalysisServerTestMixin.handleExpectedRequest.<fn>
dart:async/zone.dart 1538:47 _rootRunUnary
dart:async/zone.dart 1429:19 _CustomZone.runUnary
dart:async/future_impl.dart 229:22 _FutureListener.handleError
dart:async/future_impl.dart 944:47 Future._propagateToListeners.handleError
dart:async/future_impl.dart 965:13 Future._propagateToListeners
dart:async/future_impl.dart 730:5 Future._completeError
```
Additionally, if the timeout is not because of a failed outbound request, the error message is now like "Did not receive the expected workspace/applyEdit request from the server in the timeout period" instead of "Future not completed".
Fixes https://github.com/dart-lang/sdk/issues/59780
Change-Id: I8b8cc25194390ffffbe034eae313504792a42211
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403460
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart
index fd8512a..ca6b503 100644
--- a/pkg/analysis_server/test/lsp/server_abstract.dart
+++ b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -1134,7 +1134,15 @@
var firstRequest = requestsFromServer.firstWhere((n) => n.method == method);
await f();
- var requestFromServer = await firstRequest.timeout(timeout);
+ var requestFromServer = await firstRequest.timeout(
+ timeout,
+ onTimeout:
+ () =>
+ throw TimeoutException(
+ 'Did not receive the expected $method request from the server in the timeout period',
+ timeout,
+ ),
+ );
expect(requestFromServer, isNotNull);
return requestFromServer;
@@ -1174,6 +1182,7 @@
Duration timeout = const Duration(seconds: 5),
}) async {
late Future<T> outboundRequest;
+ Object? outboundRequestError;
// Run [f] and wait for the incoming request from the server.
var incomingRequest = await expectRequest(method, () {
@@ -1182,15 +1191,23 @@
outboundRequest = f();
// Because we don't await this future until "later", if it throws the
- // error is treated as unhandled and will fail the test. Attaching an
- // error handler prevents that, though since the Future completed with
- // an error it will still be handled as such when the future is later
- // awaited.
-
- // TODO(srawlins): Fix this static error.
- // ignore: body_might_complete_normally_catch_error
- outboundRequest.catchError((_) {});
- });
+ // error is treated as unhandled and will fail the test even if expected.
+ // Instead, capture the error and suppress it. But if we time out (in
+ // which case we will never return outboundRequest), then we'll raise this
+ // error.
+ outboundRequest.then(
+ (_) {},
+ onError: (e) {
+ outboundRequestError = e;
+ return null;
+ },
+ );
+ }, timeout: timeout).catchError((Object timeoutException) {
+ // We timed out waiting for the request from the server. Probably this is
+ // because our outbound request for some reason, so if we have an error
+ // for that, then throw it. Otherwise, propogate the timeout.
+ throw outboundRequestError ?? timeoutException;
+ }, test: (e) => e is TimeoutException);
// Handle the request from the server and send the response back.
var clientsResponse = await handler(
