sdk

sdk copied to clipboard

See the generalized description of the issue in https://github.com/dart-lang/sdk/issues/49582.

The actual error happened after migration of dwds to null safety. The culprit was DwdsVmClient passing a sink from StreamController<Map<String, Object>> to the constructor of VmServerConnection:

    final requestController = StreamController<Map<String, Object>>();
    final responseController = StreamController<Map<String, Object>>();
    VmServerConnection(requestController.stream, responseController.sink,
        debugService.serviceExtensionRegistry, debugService.chromeProxyService);

The solution was to change the declaration of the response controller to final responseController = StreamController<Map<String, Object?>>(); (allowing null value types) but the error can be only discovered at runtime when the VmServiceConnection._delegateRequest function tries to add to the sink, which is far away from the callsite of the constructor. The exception stack trace does not contain any useful information at this point:

In our dwds CI (https://github.com/dart-lang/webdev/runs/7622869869?check_suite_focus=true), the error happens far from the creation of the stream controller (another library) and traces look like:

❌ test/handlers/asset_handler_test.dart: Asset handler can read large number of resources simultaneously (failed)
  Retry: Asset handler can read large number of resources simultaneously
  type '_InternalLinkedHashMap<String, Object?>' is not a subtype of type 'Map<String, Object>' of 'data'
  dart:async                                      _StreamSinkWrapper.add
  package:vm_service/src/vm_service.dart 1732:21  VmServerConnection._delegateRequest

Suggestion

Until we have a use-site variance feature on dart, would it be possible to add the following type check to the constructor of VmServiceConnection (it will still fail only at runtime but the stack trace will include the callsite of the constructor, which will make it much more easier to investigate and fix the problem if it happens again:

if (sink is StreamSink<Map<String, Object>>) throw StateError('Sink passed to VmServiceConnection does not allow null values');

/cc @bkonyi

The fix was reverted due to the following failure in g3:

Bad state: The StreamSink passed to VmServiceConnection does not allow for null values
** Note that stack traces are truncated. See [http://go/dart-chain-stack-traces](https://www.google.com/url?q=http://go/dart-chain-stack-traces&sa=D) for information on how to view the full stack trace **
package:vm_service/src/vm_service.dart 1368:7             new VmServerConnection
package:dwds/src/dwds_vm_client.dart 53:5                 DwdsVmClient.create
package:dwds/src/handlers/dev_handler.dart 512:45         DevHandler._createAppDebugServices
package:dwds/src/handlers/dev_handler.dart 239:27         DevHandler.loadAppServices

FYI @annagrin @elliette, it looks like we're still providing the wrong stream sink type arguments in DWDS, so we can't land a fix in package:vm_service until that's resolved.