firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[ tool ] Fix expression evaluation not handling errors correctly (#159151)

Browse Source 
Also starts DDS for integration test devices.

Fixes https://github.com/flutter/flutter/issues/157922
This commit is contained in:
Ben Konyi 2024-11-25 10:04:09 -05:00 committed by GitHub
parent cca41301f0
commit b5331b3e1b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 166 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
packages/flutter_tools/lib/src/compile.dart
View File

@ -61,7 +61,7 @@ class TargetModel {
}
class CompilerOutput {
const CompilerOutput(this.outputFilename, this.errorCount, this.sources, {this.expressionData});
const CompilerOutput(this.outputFilename, this.errorCount, this.sources, {this.expressionData, this.errorMessage});
final String outputFilename;
final int errorCount;
@ -69,6 +69,9 @@ class CompilerOutput {
/// This field is only non-null for expression compilation requests.
final Uint8List? expressionData;
/// This field is only non-null when a compilation error was encountered.
final String? errorMessage;
}
enum StdoutState { CollectDiagnostic, CollectDependencies }
@ -94,6 +97,7 @@ class StdoutHandler {
bool _suppressCompilerMessages = false;
bool _expectSources = true;
bool _readFile = false;
StringBuffer _errorBuffer = StringBuffer();
void handler(String message) {
const String kResultPrefix = 'result ';
@ -125,6 +129,7 @@ class StdoutHandler {
errorCount,
sources,
expressionData: expressionData,
errorMessage: _errorBuffer.isNotEmpty ? _errorBuffer.toString() : null,
);
compilerOutput?.complete(output);
return;
@ -132,8 +137,10 @@ class StdoutHandler {
switch (state) {
case StdoutState.CollectDiagnostic when _suppressCompilerMessages:
_logger.printTrace(message);
_errorBuffer.writeln(message);
case StdoutState.CollectDiagnostic:
_logger.printError(message);
_errorBuffer.writeln(message);
case StdoutState.CollectDependencies:
switch (message[0]) {
case '+':
@ -155,6 +162,7 @@ class StdoutHandler {
_expectSources = expectSources;
_readFile = readFile;
state = StdoutState.CollectDiagnostic;
_errorBuffer = StringBuffer();
}
}

8
packages/flutter_tools/lib/src/run_hot.dart
View File

@ -209,8 +209,12 @@ class HotRunner extends ResidentRunner {
await device.generator!.compileExpression(expression, definitions,
definitionTypes, typeDefinitions, typeBounds, typeDefaults,
libraryUri, klass, method, isStatic);
if (compilerOutput != null && compilerOutput.expressionData != null) {
return base64.encode(compilerOutput.expressionData!);
if (compilerOutput != null) {
if (compilerOutput.errorCount == 0 && compilerOutput.expressionData != null) {
return base64.encode(compilerOutput.expressionData!);
} else if (compilerOutput.errorCount > 0 && compilerOutput.errorMessage != null) {
throw VmServiceExpressionCompilationException(compilerOutput.errorMessage!);
}
}
}
}

9
packages/flutter_tools/lib/src/test/flutter_platform.dart
View File

@ -27,6 +27,7 @@ import '../native_assets.dart';
import '../project.dart';
import '../test/test_wrapper.dart';
import '../vmservice.dart';
import 'flutter_tester_device.dart';
import 'font_config_manager.dart';
import 'integration_test_device.dart';
@ -425,8 +426,12 @@ class FlutterPlatform extends PlatformPlugin {
await compiler!.compiler!.compileExpression(expression, definitions,
definitionTypes, typeDefinitions, typeBounds, typeDefaults, libraryUri,
klass, method, isStatic);
if (compilerOutput != null && compilerOutput.expressionData != null) {
return base64.encode(compilerOutput.expressionData!);
if (compilerOutput != null) {
if (compilerOutput.errorCount == 0 && compilerOutput.expressionData != null) {
return base64.encode(compilerOutput.expressionData!);
} else if (compilerOutput.errorCount > 0 && compilerOutput.errorMessage != null) {
throw VmServiceExpressionCompilationException(compilerOutput.errorMessage!);
}
}
throw Exception('Failed to compile $expression');
}

1
packages/flutter_tools/lib/src/test/flutter_tester_device.dart
View File

@ -176,7 +176,6 @@ class FlutterTesterTestDevice extends TestDevice {
if (debuggingOptions.enableDds) {
logger.printTrace('test $id: Starting Dart Development Service');
await _ddsLauncher.startDartDevelopmentServiceFromDebuggingOptions(
detectedUri,
debuggingOptions: debuggingOptions,

17
packages/flutter_tools/lib/src/test/integration_test_device.dart
View File

@ -8,6 +8,7 @@ import 'package:stream_channel/stream_channel.dart';
import 'package:vm_service/vm_service.dart' as vm_service;
import '../application_package.dart';
import '../base/dds.dart';
import '../build_info.dart';
import '../device.dart';
import '../globals.dart' as globals;
@ -32,6 +33,7 @@ class IntegrationTestTestDevice implements TestDevice {
final DebuggingOptions debuggingOptions;
final String? userIdentifier;
final CompileExpression? compileExpression;
late final DartDevelopmentService _ddsLauncher = DartDevelopmentService(logger: globals.logger);
ApplicationPackage? _applicationPackage;
final Completer<void> _finished = Completer<void>();
@ -62,7 +64,7 @@ class IntegrationTestTestDevice implements TestDevice {
if (!launchResult.started) {
throw TestDeviceException('Unable to start the app on the device.', StackTrace.current);
}
final Uri? vmServiceUri = launchResult.vmServiceUri;
Uri? vmServiceUri = launchResult.vmServiceUri;
if (vmServiceUri == null) {
throw TestDeviceException('The VM Service is not available on the test device.', StackTrace.current);
}
@ -70,11 +72,21 @@ class IntegrationTestTestDevice implements TestDevice {
// No need to set up the log reader because the logs are captured and
// streamed to the package:test_core runner.
if (debuggingOptions.enableDds) {
globals.printTrace('test $id: Starting Dart Development Service');
await _ddsLauncher.startDartDevelopmentServiceFromDebuggingOptions(
vmServiceUri,
debuggingOptions: debuggingOptions,
);
globals.printTrace('test $id: Dart Development Service started at ${_ddsLauncher.uri}, forwarding to VM service at $vmServiceUri.');
vmServiceUri = _ddsLauncher.uri;
}
_gotProcessVmServiceUri.complete(vmServiceUri);
globals.printTrace('test $id: Connecting to vm service');
final FlutterVmService vmService = await connectToVmService(
vmServiceUri,
vmServiceUri!,
logger: globals.logger,
compileExpression: compileExpression,
).timeout(
@ -131,6 +143,7 @@ class IntegrationTestTestDevice implements TestDevice {
}
await device.dispose();
_ddsLauncher.shutdown();
_finished.complete();
}

39
packages/flutter_tools/lib/src/vmservice.dart
View File

@ -21,6 +21,7 @@ import 'version.dart';
const String kResultType = 'type';
const String kResultTypeSuccess = 'Success';
const String kError = 'error';
const String kGetSkSLsMethod = '_flutter.getSkSLs';
const String kSetAssetBundlePathMethod = '_flutter.setAssetBundlePath';
@ -245,13 +246,29 @@ Future<vm_service.VmService> setUpVmService({
final String? method = params['method'] as String?;
final bool isStatic = _validateRpcBoolParam('compileExpression', params, 'isStatic');
final String kernelBytesBase64 = await compileExpression(isolateId,
expression, definitions, definitionTypes, typeDefinitions, typeBounds, typeDefaults,
libraryUri, klass, method, isStatic);
return <String, Object>{
kResultType: kResultTypeSuccess,
'result': <String, String>{'kernelBytes': kernelBytesBase64},
};
try {
final String kernelBytesBase64 = await compileExpression(isolateId,
expression, definitions, definitionTypes, typeDefinitions, typeBounds, typeDefaults,
libraryUri, klass, method, isStatic);
return <String, Object>{
kResultType: kResultTypeSuccess,
'result': <String, String>{'kernelBytes': kernelBytesBase64},
};
} on VmServiceExpressionCompilationException catch (e) {
// In most situations, we'd just let VmService catch this exception and
// build the error response. However, in this case we build the error
// response manually and return it to avoid including the stack trace
// from the tool in the response, instead returning the compilation
// error message in the 'details' property of the returned error object.
return <String, Object>{
kError: vm_service.RPCError.withDetails(
'compileExpression',
vm_service.RPCErrorKind.kExpressionCompilationError.code,
vm_service.RPCErrorKind.kExpressionCompilationError.message,
details: e.errorMessage,
).toMap(),
};
}
});
registrationRequests.add(vmService.registerService(kCompileExpressionServiceName, kFlutterToolAlias));
}
@ -1072,6 +1089,14 @@ class FlutterVmService {
/// Thrown when the VM Service disappears while calls are being made to it.
class VmServiceDisappearedException implements Exception { }
/// Thrown when the frontend service fails to compile an expression due to an
/// error.
class VmServiceExpressionCompilationException implements Exception {
const VmServiceExpressionCompilationException(this.errorMessage);
final String errorMessage;
}
/// Whether the event attached to an [Isolate.pauseEvent] should be considered
/// a "pause" event.
bool isPauseEvent(String kind) {

22
packages/flutter_tools/test/general.shard/integration_test_device_test.dart
View File

@ -4,6 +4,7 @@
import 'package:file/file.dart';
import 'package:flutter_tools/src/application_package.dart';
import 'package:flutter_tools/src/base/dds.dart';
import 'package:flutter_tools/src/base/io.dart' as io;
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart';
@ -19,6 +20,7 @@ import 'package:vm_service/vm_service.dart' as vm_service;
import '../src/context.dart';
import '../src/fake_devices.dart';
import '../src/fake_vm_services.dart';
import '../src/fakes.dart';
final vm_service.Isolate isolate = vm_service.Isolate(
id: '1',
@ -64,6 +66,7 @@ final Uri vmServiceUri = Uri.parse('http://localhost:1234');
void main() {
late FakeVmServiceHost fakeVmServiceHost;
late TestDevice testDevice;
late DDSLauncherCallback originalDdsLauncher;
setUp(() {
testDevice = IntegrationTestTestDevice(
@ -103,6 +106,25 @@ void main() {
},
),
]);
originalDdsLauncher = ddsLauncherCallback;
ddsLauncherCallback = ({
required Uri remoteVmServiceUri,
Uri? serviceUri,
bool enableAuthCodes = true,
bool serveDevTools = false,
Uri? devToolsServerAddress,
bool enableServicePortFallback = false,
List<String> cachedUserTags = const <String>[],
String? dartExecutable,
String? google3WorkspaceRoot,
}) async {
return FakeDartDevelopmentServiceLauncher(uri: Uri.parse('http://localhost:1234'));
};
});
tearDown(() {
ddsLauncherCallback = originalDdsLauncher;
});
testUsingContext('will not start when package missing', () async {

122
packages/flutter_tools/test/integration.shard/expression_evaluation_test.dart
View File

@ -18,16 +18,16 @@ void batch1() {
late Directory tempDir;
late FlutterRunTestDriver flutter;
Future<void> initProject() async {
setUp(() async {
tempDir = createResolvedTempDirectorySync('run_expression_eval_test.');
await project.setUpIn(tempDir);
flutter = FlutterRunTestDriver(tempDir);
}
});
Future<void> cleanProject() async {
tearDown(() async {
await flutter.stop();
tryToDelete(tempDir);
}
});
Future<void> breakInBuildMethod(FlutterTestDriver flutter) async {
await flutter.breakAt(
@ -43,52 +43,46 @@ void batch1() {
);
}
testWithoutContext('flutter run expression evaluation - can evaluate trivial expressions in top level function', () async {
await initProject();
testWithoutContext('can evaluate trivial expressions in top level function', () async {
await flutter.run(withDebugger: true);
await breakInTopLevelFunction(flutter);
await evaluateTrivialExpressions(flutter);
await cleanProject();
});
testWithoutContext('flutter run expression evaluation - can evaluate trivial expressions in build method', () async {
await initProject();
testWithoutContext('can evaluate trivial expressions in build method', () async {
await flutter.run(withDebugger: true);
await breakInBuildMethod(flutter);
await evaluateTrivialExpressions(flutter);
await cleanProject();
});
testWithoutContext('flutter run expression evaluation - can evaluate complex expressions in top level function', () async {
await initProject();
testWithoutContext('can evaluate complex expressions in top level function', () async {
await flutter.run(withDebugger: true);
await breakInTopLevelFunction(flutter);
await evaluateComplexExpressions(flutter);
await cleanProject();
});
testWithoutContext('flutter run expression evaluation - can evaluate complex expressions in build method', () async {
await initProject();
testWithoutContext('can evaluate complex expressions in build method', () async {
await flutter.run(withDebugger: true);
await breakInBuildMethod(flutter);
await evaluateComplexExpressions(flutter);
await cleanProject();
});
testWithoutContext('flutter run expression evaluation - can evaluate expressions returning complex objects in top level function', () async {
await initProject();
testWithoutContext('can evaluate expressions returning complex objects in top level function', () async {
await flutter.run(withDebugger: true);
await breakInTopLevelFunction(flutter);
await evaluateComplexReturningExpressions(flutter);
await cleanProject();
});
testWithoutContext('flutter run expression evaluation - can evaluate expressions returning complex objects in build method', () async {
await initProject();
testWithoutContext('can evaluate expressions returning complex objects in build method', () async {
await flutter.run(withDebugger: true);
await breakInBuildMethod(flutter);
await evaluateComplexReturningExpressions(flutter);
await cleanProject();
});
testWithoutContext('evaluating invalid expressions throws an exception with compilation error details', () async {
await flutter.run(withDebugger: true);
await breakInTopLevelFunction(flutter);
await evaluateInvalidExpression(flutter);
});
}
@ -97,19 +91,18 @@ void batch2() {
late Directory tempDir;
late FlutterTestTestDriver flutter;
Future<void> initProject() async {
setUp(() async {
tempDir = createResolvedTempDirectorySync('test_expression_eval_test.');
await project.setUpIn(tempDir);
flutter = FlutterTestTestDriver(tempDir);
}
});
Future<void> cleanProject() async {
tearDown(() async {
await flutter.waitForCompletion();
tryToDelete(tempDir);
}
});
testWithoutContext('flutter test expression evaluation - can evaluate trivial expressions in a test', () async {
await initProject();
testWithoutContext('can evaluate trivial expressions in a test', () async {
await flutter.test(
withDebugger: true,
beforeStart: () => flutter.addBreakpoint(project.breakpointUri, project.breakpointLine),
@ -121,30 +114,33 @@ void batch2() {
// https://github.com/Dart-Code/Dart-Code/issues/4243.
final String dillFilename = '${project.testFilePath}.dill';
expect(fileSystem.file(dillFilename).existsSync(), isFalse);
await cleanProject();
});
testWithoutContext('flutter test expression evaluation - can evaluate complex expressions in a test', () async {
await initProject();
testWithoutContext('can evaluate complex expressions in a test', () async {
await flutter.test(
withDebugger: true,
beforeStart: () => flutter.addBreakpoint(project.breakpointUri, project.breakpointLine),
);
await flutter.waitForPause();
await evaluateComplexExpressions(flutter);
await cleanProject();
});
testWithoutContext('flutter test expression evaluation - can evaluate expressions returning complex objects in a test', () async {
await initProject();
testWithoutContext('can evaluate expressions returning complex objects in a test', () async {
await flutter.test(
withDebugger: true,
beforeStart: () => flutter.addBreakpoint(project.breakpointUri, project.breakpointLine),
);
await flutter.waitForPause();
await evaluateComplexReturningExpressions(flutter);
await cleanProject();
});
testWithoutContext('evaluating invalid expressions throws an exception with compilation error details', () async {
await flutter.test(
withDebugger: true,
beforeStart: () => flutter.addBreakpoint(project.breakpointUri, project.breakpointLine),
);
await flutter.waitForPause();
await evaluateInvalidExpression(flutter);
});
}
@ -153,19 +149,18 @@ void batch3() {
late Directory tempDir;
late FlutterTestTestDriver flutter;
Future<void> initProject() async {
setUp(() async {
tempDir = createResolvedTempDirectorySync('integration_test_expression_eval_test.');
await project.setUpIn(tempDir);
flutter = FlutterTestTestDriver(tempDir);
}
});
Future<void> cleanProject() async {
tearDown(() async {
await flutter.waitForCompletion();
tryToDelete(tempDir);
}
});
testWithoutContext('flutter integration test expression evaluation - can evaluate expressions in a test', () async {
await initProject();
testWithoutContext('can evaluate expressions in a test', () async {
await flutter.test(
deviceId: 'flutter-tester',
testFile: project.testFilePath,
@ -179,10 +174,23 @@ void batch3() {
// https://github.com/Dart-Code/Dart-Code/issues/4243.
final String dillFilename = '${project.testFilePath}.dill';
expect(fileSystem.file(dillFilename).existsSync(), isFalse);
await cleanProject();
});
testWithoutContext('evaluating invalid expressions throws an exception with compilation error details', () async {
await flutter.test(
deviceId: 'flutter-tester',
testFile: project.testFilePath,
withDebugger: true,
beforeStart: () => flutter.addBreakpoint(project.breakpointUri, project.breakpointLine),
);
await flutter.waitForPause();
await evaluateInvalidExpression(flutter);
// Ensure we did not leave a dill file alongside the test.
// https://github.com/Dart-Code/Dart-Code/issues/4243.
final String dillFilename = '${project.testFilePath}.dill';
expect(fileSystem.file(dillFilename).existsSync(), isFalse);
});
}
Future<void> evaluateTrivialExpressions(FlutterTestDriver flutter) async {
@ -211,6 +219,26 @@ Future<void> evaluateComplexReturningExpressions(FlutterTestDriver flutter) asyn
expectValue(res, '${date.year}-${date.month}-${date.day}');
}
Future<void> evaluateInvalidExpression(FlutterTestDriver flutter) async {
try {
await flutter.evaluateInFrame('is Foo');
fail("'is Foo' is not a valid expression");
} on RPCError catch (e) {
expect(e.code, RPCErrorKind.kExpressionCompilationError.code);
expect(e.message, RPCErrorKind.kExpressionCompilationError.message);
expect(
e.details,
"org-dartlang-debug:synthetic_debug_expression:1:1: Error: Expected an identifier, but got 'is'.\n"
"Try inserting an identifier before 'is'.\n"
'is Foo\n'
'^^\n'
"org-dartlang-debug:synthetic_debug_expression:1:4: Error: 'Foo' isn't a type.\n"
'is Foo\n'
' ^^^\n',
);
}
}
void expectInstanceOfClass(ObjRef result, String name) {
expect(result,
const TypeMatcher<InstanceRef>()
@ -231,7 +259,7 @@ void expectValue(ObjRef result, String message) {
}
void main() {
batch1();
batch2();
batch3();
group('flutter run expression evaluation -', batch1);
group('flutter test expression evaluation -', batch2);
group('flutter integration test expression evaluation -', batch3);
}