Skip to content

Commit fc54d68

Browse files
author
Anna Gringauze
authored
Fix expression evaluation failures on empty scopes (#1998)
1 parent 496a2b4 commit fc54d68

File tree

4 files changed

+43
-1
lines changed

4 files changed

+43
-1
lines changed

Diff for: dwds/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
- Remove test-only code from `sdk_configuration.dart`.
1313
- Move shared test-only code to a new `test_common` package.
1414
- Convert unnecessary async code to sync.
15+
- Allow empty scopes in expression evaluation in a frame.
1516

1617
**Breaking changes**
1718

Diff for: dwds/lib/src/services/expression_evaluator.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ class ExpressionEvaluator {
143143
/// [expression] dart expression to evaluate.
144144
Future<RemoteObject> evaluateExpressionInFrame(String isolateId,
145145
int frameIndex, String expression, Map<String, String>? scope) async {
146-
if (scope != null) {
146+
if (scope != null && scope.isNotEmpty) {
147147
// TODO(annagrin): Implement scope support.
148148
// Issue: https://github.com/dart-lang/webdev/issues/1344
149149
return createError(

Diff for: dwds/test/expression_evaluator_test.dart

+36
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import 'package:dwds/src/services/expression_evaluator.dart';
1616
import 'package:test/test.dart';
1717
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
1818

19+
import 'fixtures/context.dart';
1920
import 'fixtures/fakes.dart';
2021

2122
late ExpressionEvaluator? _evaluator;
@@ -94,6 +95,41 @@ void main() async {
9495
.having((o) => o.value, 'value', 'true'));
9596
});
9697

98+
test('can evaluate expression in frame with null scope', () async {
99+
// Verify that we don't get the internal error.
100+
// More extensive testing of 'evaluateExpressionInFrame' is done in
101+
// evaluation tests for frontend server and build daemon.
102+
await expectLater(
103+
evaluator.evaluateExpressionInFrame('1', 0, 'true', null),
104+
throwsRPCErrorWithMessage(
105+
'Cannot evaluate on a call frame when the program is not paused'));
106+
});
107+
108+
test('can evaluate expression in frame with empty scope', () async {
109+
// Verify that we don't get the internal error.
110+
// More extensive testing of 'evaluateExpressionInFrame' is done in
111+
// evaluation tests for frontend server and build daemon.
112+
await expectLater(
113+
evaluator.evaluateExpressionInFrame('1', 0, 'true', {}),
114+
throwsRPCErrorWithMessage(
115+
'Cannot evaluate on a call frame when the program is not paused'));
116+
});
117+
118+
test('cannot evaluate expression in frame with non-empty scope',
119+
() async {
120+
final result = await evaluator
121+
.evaluateExpressionInFrame('1', 0, 'true', {'a': '1'});
122+
expect(
123+
result,
124+
const TypeMatcher<RemoteObject>()
125+
.having((o) => o.type, 'type', 'InternalError')
126+
.having(
127+
(o) => o.value,
128+
'value',
129+
contains(
130+
'Using scope for expression evaluation in frame is not supported')));
131+
});
132+
97133
test('returns error if closed', () async {
98134
evaluator.close();
99135
final result =

Diff for: dwds/test/fixtures/context.dart

+5
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ const isSentinelException = TypeMatcher<SentinelException>();
4949
final Matcher throwsRPCError = throwsA(isRPCError);
5050
final Matcher throwsSentinelException = throwsA(isSentinelException);
5151

52+
Matcher isRPCErrorWithMessage(String message) =>
53+
isA<RPCError>().having((e) => e.message, 'message', contains(message));
54+
Matcher throwsRPCErrorWithMessage(String message) =>
55+
throwsA(isRPCErrorWithMessage(message));
56+
5257
enum CompilationMode { buildDaemon, frontendServer }
5358

5459
class TestContext {

0 commit comments

Comments
 (0)