Skip to content

Commit c67d258

Browse files
authored
Provide more useful error message if a non-compliant DAP tool (or user) sends bad input to DAP server (#107827)
1 parent 0d40e3d commit c67d258

File tree

7 files changed

+58
-10
lines changed

7 files changed

+58
-10
lines changed

packages/flutter_tools/lib/src/commands/debug_adapter.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ class DebugAdapterCommand extends FlutterCommand {
5959
ipv6: ipv6 ?? false,
6060
enableDds: enableDds,
6161
test: boolArgDeprecated('test'),
62+
onError: (Object? e) {
63+
globals.printError(
64+
'Input could not be parsed as a Debug Adapter Protocol message.\n'
65+
'The "flutter debug-adapter" command is intended for use by tooling '
66+
'that communicates using the Debug Adapter Protocol.\n\n'
67+
'$e',
68+
);
69+
},
6270
);
6371

6472
await server.channel.closed;

packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class FlutterDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArguments
2828
bool enableDds = true,
2929
super.enableAuthCodes,
3030
super.logger,
31+
super.onError,
3132
}) : _enableDds = enableDds,
3233
// Always disable in the DAP layer as it's handled in the spawned
3334
// 'flutter' process.

packages/flutter_tools/lib/src/debug_adapters/flutter_test_adapter.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class FlutterTestDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArgum
2828
bool enableDds = true,
2929
super.enableAuthCodes,
3030
super.logger,
31+
super.onError,
3132
}) : _enableDds = enableDds,
3233
// Always disable in the DAP layer as it's handled in the spawned
3334
// 'flutter' process.

packages/flutter_tools/lib/src/debug_adapters/server.dart

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,28 @@ class DapServer {
3030
this.enableAuthCodes = true,
3131
bool test = false,
3232
this.logger,
33+
void Function(Object? e)? onError,
3334
}) : channel = ByteStreamServerChannel(input, output, logger) {
3435
adapter = test
35-
? FlutterTestDebugAdapter(channel,
36+
? FlutterTestDebugAdapter(
37+
channel,
3638
fileSystem: fileSystem,
3739
platform: platform,
3840
ipv6: ipv6,
3941
enableDds: enableDds,
4042
enableAuthCodes: enableAuthCodes,
41-
logger: logger)
42-
: FlutterDebugAdapter(channel,
43+
logger: logger,
44+
onError: onError,
45+
)
46+
: FlutterDebugAdapter(
47+
channel,
4348
fileSystem: fileSystem,
4449
platform: platform,
4550
enableDds: enableDds,
4651
enableAuthCodes: enableAuthCodes,
47-
logger: logger);
52+
logger: logger,
53+
onError: onError,
54+
);
4855
}
4956

5057
final ByteStreamServerChannel channel;

packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@ import 'package:dds/dap.dart';
88
import 'package:dds/src/dap/protocol_generated.dart';
99
import 'package:file/file.dart';
1010
import 'package:flutter_tools/src/cache.dart';
11+
import 'package:flutter_tools/src/convert.dart';
1112
import 'package:flutter_tools/src/globals.dart' as globals;
1213

1314
import '../../src/common.dart';
1415
import '../test_data/basic_project.dart';
1516
import '../test_data/compile_error_project.dart';
1617
import '../test_utils.dart';
1718
import 'test_client.dart';
19+
import 'test_server.dart';
1820
import 'test_support.dart';
1921

2022
void main() {
@@ -97,6 +99,27 @@ void main() {
9799
]);
98100
});
99101

102+
testWithoutContext('outputs useful message on invalid DAP protocol messages', () async {
103+
final OutOfProcessDapTestServer server = dap.server as OutOfProcessDapTestServer;
104+
final CompileErrorProject project = CompileErrorProject();
105+
await project.setUpIn(tempDir);
106+
107+
final StringBuffer stderrOutput = StringBuffer();
108+
dap.server.onStderrOutput = stderrOutput.write;
109+
110+
// Write invalid headers and await the error.
111+
dap.server.sink.add(utf8.encode('foo\r\nbar\r\n\r\n'));
112+
await server.exitCode;
113+
114+
// Verify the user-friendly message was included in the output.
115+
final String error = stderrOutput.toString();
116+
expect(error, contains('Input could not be parsed as a Debug Adapter Protocol message'));
117+
expect(error, contains('The "flutter debug-adapter" command is intended for use by tooling'));
118+
// This test only runs with out-of-process DAP as it's testing _actual_
119+
// stderr output and that the debug-adapter process terminates, which is
120+
// not possible when running the DAP Server in-process.
121+
}, skip: useInProcessDap); // [intended] See above.
122+
100123
testWithoutContext('correctly outputs launch errors and terminates', () async {
101124
final CompileErrorProject project = CompileErrorProject();
102125
await project.setUpIn(tempDir);

packages/flutter_tools/test/integration.shard/debug_adapter/test_client.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ extension DapTestClientExtension on DapTestClient {
325325
Future<List<OutputEventBody>> collectAllOutput({
326326
String? program,
327327
String? cwd,
328-
Future<Response> Function()? start,
328+
Future<void> Function()? start,
329329
Future<Response> Function()? launch,
330330
bool skipInitialPubGetOutput = true
331331
}) async {

packages/flutter_tools/test/integration.shard/debug_adapter/test_server.dart

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ abstract class DapTestServer {
1919
Future<void> stop();
2020
StreamSink<List<int>> get sink;
2121
Stream<List<int>> get stream;
22+
Function(String message)? onStderrOutput;
2223
}
2324

2425
/// An instance of a DAP server running in-process (to aid debugging).
@@ -73,22 +74,27 @@ class OutOfProcessDapTestServer extends DapTestServer {
7374
this._process,
7475
Logger? logger,
7576
) {
76-
// Treat anything written to stderr as the DAP crashing and fail the test
77-
// unless it's "Waiting for another flutter command to release the startup
78-
// lock" or we're tearing down.
77+
// Unless we're given an error handler, treat anything written to stderr as
78+
// the DAP crashing and fail the test unless it's "Waiting for another
79+
// flutter command to release the startup lock" or we're tearing down.
7980
_process.stderr
8081
.transform(utf8.decoder)
8182
.where((String error) => !error.contains('Waiting for another flutter command to release the startup lock'))
8283
.listen((String error) {
8384
logger?.call(error);
8485
if (!_isShuttingDown) {
85-
throw Exception(error);
86+
final Function(String message)? stderrHandler = onStderrOutput;
87+
if (stderrHandler != null) {
88+
stderrHandler(error);
89+
} else {
90+
throw Exception(error);
91+
}
8692
}
8793
});
8894
unawaited(_process.exitCode.then((int code) {
8995
final String message = 'Out-of-process DAP server terminated with code $code';
9096
logger?.call(message);
91-
if (!_isShuttingDown && code != 0) {
97+
if (!_isShuttingDown && code != 0 && onStderrOutput == null) {
9298
throw Exception(message);
9399
}
94100
}));
@@ -97,6 +103,8 @@ class OutOfProcessDapTestServer extends DapTestServer {
97103
bool _isShuttingDown = false;
98104
final Process _process;
99105

106+
Future<int> get exitCode => _process.exitCode;
107+
100108
@override
101109
StreamSink<List<int>> get sink => _process.stdin;
102110

0 commit comments

Comments
 (0)