[Property Editor] Handle errors from the Analysis Server (#8818)

packages/devtools_app/benchmark/web_bundle_size_test.dart

@@ -12,7 +12,7 @@ import 'package:test/test.dart';
 
 // Benchmark size in kB.
 const bundleSizeBenchmark = 5400;
-const gzipBundleSizeBenchmark = 1550;
+const gzipBundleSizeBenchmark = 1650;
 
 void main() {
   group('Web Compile', () {

packages/devtools_app/lib/src/shared/editor/api_classes.dart

@@ -27,6 +27,9 @@ enum EditorMethod {
 /// [pkg/analysis_server/lib/src/lsp/constants.dart][code link]
 ///
 /// [code link]: https://github.com/dart-lang/sdk/blob/ebfcd436da65802a2b20d415afe600b51e432305/pkg/analysis_server/lib/src/lsp/constants.dart#L136
+///
+/// TODO(https://github.com/flutter/devtools/issues/8824): Add tests that these
+/// are in-sync with analysis_server.
 enum LspMethod {
   editableArguments(methodName: 'dart/textDocument/editableArguments'),
   editArgument(methodName: 'dart/textDocument/editArgument');
@@ -442,6 +445,71 @@ class EditableArgumentsResult with Serializable {
   Map<String, Object?> toJson() => {Field.arguments: args};
 }
 
+/// Errors that the Analysis Server returns for failed argument edits.
+///
+/// These should be kept in sync with the error coes defined at
+/// [pkg/analysis_server/lib/src/lsp/constants.dart][code link]
+///
+/// [code link]: https://github.com/dart-lang/sdk/blob/35a10987e1652b7d49991ab2dc2ee7f521fe8d8f/pkg/analysis_server/lib/src/lsp/constants.dart#L300
+///
+/// TODO(https://github.com/flutter/devtools/issues/8824): Add tests that these
+/// are in-sync with analysis_server.
+enum EditArgumentError {
+  /// A request was made that requires use of workspace/applyEdit but the
+  /// current editor does not support it.
+  editsUnsupportedByEditor(
+    code: -32016,
+    message: 'IDE does not support property edits.',
+  ),
+
+  /// An editArgument request tried to modify an invocation at a position where
+  /// there was no invocation.
+  editArgumentInvalidPosition(
+    code: -32017,
+    message: 'Invalid position for argument.',
+  ),
+
+  /// An editArgument request tried to modify a parameter that does not exist or
+  /// is not editable.
+  editArgumentInvalidParameter(code: -32018, message: 'Invalid parameter.'),
+
+  /// An editArgument request tried to set an argument value that is not valid.
+  editArgumentInvalidValue(
+    code: -32019,
+    message: 'Invalid value for parameter.',
+  );
+
+  const EditArgumentError({required this.code, required this.message});
+
+  final int code;
+  final String message;
+
+  static final _codeToErrorMap = EditArgumentError.values.fold(
+    <int, EditArgumentError>{},
+    (map, error) {
+      map[error.code] = error;
+      return map;
+    },
+  );
+
+  static EditArgumentError? fromCode(int? code) {
+    if (code == null) return null;
+    return _codeToErrorMap[code];
+  }
+}
+
+/// Response to an edit argument request.
+class EditArgumentResponse {
+  EditArgumentResponse({required this.success, this.errorMessage, errorCode})
+    : _errorCode = errorCode;
+
+  final bool success;
+  final String? errorMessage;
+  final int? _errorCode;
+
+  EditArgumentError? get errorType => EditArgumentError.fromCode(_errorCode);
+}
+
 /// Information about a single editable argument of a widget.
 class EditableArgument with Serializable {
   EditableArgument({

packages/devtools_app/lib/src/shared/editor/editor_client.dart

@@ -7,6 +7,7 @@ import 'dart:async';
 import 'package:devtools_app_shared/utils.dart';
 import 'package:dtd/dtd.dart';
 import 'package:flutter/foundation.dart';
+import 'package:json_rpc_2/json_rpc_2.dart';
 import 'package:logging/logging.dart';
 
 import '../analytics/constants.dart';
@@ -271,26 +272,46 @@ class EditorClient extends DisposableController
   }
 
   /// Requests that the Analysis Server makes a code edit for an argument.
-  Future<void> editArgument<T>({
+  Future<EditArgumentResponse> editArgument<T>({
     required TextDocument textDocument,
     required CursorPosition position,
     required String name,
     required T value,
   }) async {
     final method = editArgumentMethodName.value;
-    if (method == null) return;
-    final response = await _callLspApi(
-      method,
-      params: {
-        'type': 'Object', // This is required by DTD.
-        'textDocument': textDocument.toJson(),
-        'position': position.toJson(),
-        'edit': {'name': name, 'newValue': value},
-      },
-    );
-    // TODO(elliette): Handle response, currently the response from the Analysis
-    // Server is null.
-    _log.info('editArgument response: ${response.result}');
+    if (method == null) {
+      return EditArgumentResponse(
+        success: false,
+        errorMessage: 'API is unavailable.',
+      );
+    }
+    try {
+      await _callLspApi(
+        method,
+        params: {
+          'type': 'Object', // This is required by DTD.
+          'textDocument': textDocument.toJson(),
+          'position': position.toJson(),
+          'edit': {'name': name, 'newValue': value},
+        },
+      );
+      return EditArgumentResponse(success: true);
+    } on RpcException catch (e) {
+      final errorMessage = e.message;
+      _log.severe(errorMessage);
+      return EditArgumentResponse(
+        success: false,
+        errorCode: e.code,
+        errorMessage: errorMessage,
+      );
+    } catch (e) {
+      final errorMessage = 'Unknown error: $e';
+      _log.severe(errorMessage);
+      return EditArgumentResponse(
+        success: false,
+        errorMessage: 'Unknown error: $e',
+      );
+    }
   }
 
   Future<DTDResponse> _call(

packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart

@@ -49,11 +49,14 @@ class PropertyEditorController extends DisposableController
     );
   }
 
-  Future<void> editArgument<T>({required String name, required T value}) async {
+  Future<EditArgumentResponse?> editArgument<T>({
+    required String name,
+    required T value,
+  }) async {
     final document = _currentDocument;
     final position = _currentCursorPosition;
-    if (document == null || position == null) return;
-    await editorClient.editArgument(
+    if (document == null || position == null) return null;
+    return editorClient.editArgument(
       textDocument: document,
       position: position,
       name: name,

packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart

@@ -6,6 +6,7 @@ import 'package:devtools_app_shared/ui.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 
+import '../../../shared/editor/api_classes.dart';
 import 'property_editor_controller.dart';
 import 'property_editor_types.dart';
 
@@ -89,48 +90,69 @@ class StringInput extends StatelessWidget {
   }
 }
 
-class _DropdownInput<T> extends StatelessWidget with _PropertyInputMixin<T> {
-  _DropdownInput({super.key, required this.property, required this.controller});
+class _DropdownInput<T> extends StatefulWidget {
+  const _DropdownInput({
+    super.key,
+    required this.property,
+    required this.controller,
+  });
 
   final FiniteValuesProperty property;
   final PropertyEditorController controller;
 
+  @override
+  State<_DropdownInput<T>> createState() => _DropdownInputState<T>();
+}
+
+class _DropdownInputState<T> extends State<_DropdownInput<T>>
+    with _PropertyInputMixin<_DropdownInput<T>, T> {
   @override
   Widget build(BuildContext context) {
     final theme = Theme.of(context);
     return DropdownButtonFormField(
-      value: property.valueDisplay,
-      decoration: decoration(property, theme: theme, padding: denseSpacing),
+      value: widget.property.valueDisplay,
+      autovalidateMode: AutovalidateMode.onUserInteraction,
+      validator: (text) => inputValidator(text, property: widget.property),
+      decoration: decoration(
+        widget.property,
+        theme: theme,
+        padding: denseSpacing,
+      ),
       isExpanded: true,
       items:
-          property.propertyOptions.map((option) {
+          widget.property.propertyOptions.map((option) {
             return DropdownMenuItem(
               value: option,
               child: Text(option, style: theme.fixedFontStyle),
             );
           }).toList(),
       onChanged: (newValue) async {
         await editProperty(
-          property,
+          widget.property,
           valueAsString: newValue,
-          controller: controller,
+          controller: widget.controller,
         );
       },
     );
   }
 }
 
-class _TextInput<T> extends StatefulWidget with _PropertyInputMixin<T> {
-  _TextInput({super.key, required this.property, required this.controller});
+class _TextInput<T> extends StatefulWidget {
+  const _TextInput({
+    super.key,
+    required this.property,
+    required this.controller,
+  });
 
   final EditableProperty property;
   final PropertyEditorController controller;
 
   @override
-  State<_TextInput> createState() => _TextInputState();
+  State<_TextInput> createState() => _TextInputState<T>();
 }
 
-class _TextInputState extends State<_TextInput> {
+class _TextInputState<T> extends State<_TextInput<T>>
+    with _PropertyInputMixin<_TextInput<T>, T> {
   String currentValue = '';
 
   double paddingDiffComparedToDropdown = 1.0;
@@ -142,9 +164,9 @@ class _TextInputState extends State<_TextInput> {
       initialValue: widget.property.valueDisplay,
       enabled: widget.property.isEditable,
       autovalidateMode: AutovalidateMode.onUserInteraction,
-      validator: widget.property.inputValidator,
+      validator: (text) => inputValidator(text, property: widget.property),
       inputFormatters: [FilteringTextInputFormatter.singleLineFormatter],
-      decoration: widget.decoration(
+      decoration: decoration(
         widget.property,
         theme: theme,
         // Note: The text input has an extra pixel compared to the dropdown
@@ -154,6 +176,7 @@ class _TextInputState extends State<_TextInput> {
       ),
       style: theme.fixedFontStyle,
       onChanged: (newValue) {
+        clearServerError();
         setState(() {
           currentValue = newValue;
         });
@@ -166,20 +189,23 @@ class _TextInputState extends State<_TextInput> {
   }
 
   Future<void> _editProperty() async {
-    await widget.editProperty(
+    await editProperty(
       widget.property,
       valueAsString: currentValue,
       controller: widget.controller,
     );
   }
 }
 
-mixin _PropertyInputMixin<T> {
+mixin _PropertyInputMixin<T extends StatefulWidget, U> on State<T> {
+  String? _serverError;
+
   Future<void> editProperty(
     EditableProperty property, {
     required PropertyEditorController controller,
     required String? valueAsString,
   }) async {
+    clearServerError();
     final argName = property.name;
 
     // Can edit values to null.
@@ -188,8 +214,9 @@ mixin _PropertyInputMixin<T> {
       return;
     }
 
-    final value = property.convertFromInputString(valueAsString) as T?;
-    await controller.editArgument(name: argName, value: value);
+    final value = property.convertFromInputString(valueAsString) as U?;
+    final response = await controller.editArgument(name: argName, value: value);
+    _maybeHandleServerError(response, property: property);
   }
 
   InputDecoration decoration(
@@ -232,4 +259,26 @@ mixin _PropertyInputMixin<T> {
       ),
     );
   }
+
+  String? inputValidator(String? input, {required EditableProperty property}) {
+    if (_serverError != null) return _serverError;
+    return property.inputValidator(input);
+  }
+
+  void clearServerError() {
+    setState(() {
+      _serverError = null;
+    });
+  }
+
+  void _maybeHandleServerError(
+    EditArgumentResponse? errorResponse, {
+    required EditableProperty property,
+  }) {
+    if (errorResponse == null || errorResponse.success) return;
+    setState(() {
+      _serverError =
+          '${errorResponse.errorType?.message ?? 'Encountered unknown error.'} (Property: ${property.name})';
+    });
+  }
 }

packages/devtools_app/pubspec.yaml

@@ -37,6 +37,7 @@ dependencies:
   http: ^1.1.0
   image: ^4.1.3
   intl: ^0.19.0
+  json_rpc_2: ^3.0.2
   logging: ^1.1.1
   meta: ^1.9.1
   mime: ^2.0.0
@@ -69,7 +70,6 @@ dev_dependencies:
     sdk: flutter
   integration_test:
     sdk: flutter
-  json_rpc_2: ^3.0.2
   mockito: ^5.4.1
   stager: ^1.0.1
   stream_channel: ^2.1.1