[dwds] callServiceExtension should check all extensions and return kMethodNotFound when extension not found (#2597)

#2584

Currently, callServiceExtension checks the extensionRPCs field in the VM service. This list is populated when DWDS starts and checks the extensions currently registered in the runtime. This list is prone to becoming stale, however, as services may be registered or removed through the runtime (via dart:developer) later.

Instead, we should seek to always check the runtime for the current registered extensions and then update this list whenever we do. It’s still prone to becoming stale, but it is less stale. Ideally, we would override extensionRPCs and fetch the current registered extensions, but that isn’t possible since it requires awaiting a future. We also can’t ignore this field altogether because it looks like clients, like Dart DevTools, uses this field. Note that extensionRPCs should be a subset of getExtensionRpcs since registerService isn't supported in DWDS.

callServiceExtension should also return kMethodNotFound instead of its current JS evaluation error when an extension is not present.

Author: srujzs

dwds/CHANGELOG.md

@@ -1,5 +1,10 @@
 ## 24.3.7-wip
 
+- `callServiceExtension` now checks the runtime for the list of registered
+  service extensions. It also now throws a `RPCError` with
+  `RPCErrorKind.kMethodNotFound` when a service extension is not found instead
+  of throwing a JS evaluation error.
+
 ## 24.3.6
 
 - Bump minimum sdk version to 3.7.0

dwds/lib/src/debugging/inspector.dart

@@ -115,8 +115,7 @@ class AppInspector implements AppInspectorInterface {
     await DartUri.initialize();
     DartUri.recordAbsoluteUris(libraries.map((lib) => lib.uri).nonNulls);
     DartUri.recordAbsoluteUris(scripts.map((script) => script.uri).nonNulls);
-
-    isolate.extensionRPCs?.addAll(await _getExtensionRpcs());
+    await getExtensionRpcs();
   }
 
   static IsolateRef _toIsolateRef(Isolate isolate) => IsolateRef(
@@ -760,11 +759,16 @@ class AppInspector implements AppInspectorInterface {
       scriptId == null ? null : _scriptRefsById[scriptId];
 
   /// Runs an eval on the page to compute all existing registered extensions.
-  Future<List<String>> _getExtensionRpcs() async {
+  ///
+  /// Combines this with the RPCs registered in the [isolate]. Use this over
+  /// [Isolate.extensionRPCs] as this computes a live set.
+  ///
+  /// Updates [Isolate.extensionRPCs] to this set.
+  Future<Set<String>> getExtensionRpcs() async {
     final expression =
         globalToolConfiguration.loadStrategy.dartRuntimeDebugger
             .getDartDeveloperExtensionNamesJsExpression();
-    final extensionRpcs = <String>[];
+    final extensionRpcs = <String>{};
     final params = {
       'expression': expression,
       'returnByValue': true,
@@ -784,6 +788,7 @@ class AppInspector implements AppInspectorInterface {
         s,
       );
     }
+    isolate.extensionRPCs = List<String>.of(extensionRpcs);
     return extensionRpcs;
   }
 

dwds/lib/src/services/chrome_proxy_service.dart

@@ -343,7 +343,7 @@ class ChromeProxyService implements VmServiceInterface {
     // TODO: We shouldn't need to fire these events since they exist on the
     // isolate, but devtools doesn't recognize extensions after a page refresh
     // otherwise.
-    for (final extensionRpc in inspector.isolate.extensionRPCs ?? []) {
+    for (final extensionRpc in await inspector.getExtensionRpcs()) {
       _streamNotify(
         'Isolate',
         Event(
@@ -509,6 +509,13 @@ class ChromeProxyService implements VmServiceInterface {
         v is String ? v : jsonEncode(v),
       ),
     );
+    if (!(await inspector.getExtensionRpcs()).contains(method)) {
+      throw RPCError(
+        method,
+        RPCErrorKind.kMethodNotFound.code,
+        'Unknown service method: $method',
+      );
+    }
     final expression = globalToolConfiguration.loadStrategy.dartRuntimeDebugger
         .invokeExtensionJsExpression(method, jsonEncode(stringArgs));
     final result = await inspector.jsEvaluate(expression, awaitPromise: true);

dwds/test/common/chrome_proxy_service_common.dart

@@ -298,6 +298,20 @@ void runTests({
           ),
         },
       );
+
+      test('not found', () async {
+        final serviceMethod = 'ext.test.missingServiceExtension';
+        expect(
+          service.callServiceExtension(serviceMethod),
+          throwsA(
+            predicate(
+              (dynamic error) =>
+                  error is RPCError &&
+                  error.code == RPCErrorKind.kMethodNotFound.code,
+            ),
+          ),
+        );
+      });
     });
 
     group('VMTimeline', () {