diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index a3bc795fb..cc7dde871 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -2,6 +2,7 @@ - Fix failure to map JS exceptions to dart. - [#2004](https://github.com/dart-lang/webdev/pull/2004) - Fix for listening to custom streams. - [#2011](https://github.com/dart-lang/webdev/pull/2011) +- Handle unexpected extension debugger disconnect events without crashing the app - [#2021](https://github.com/dart-lang/webdev/pull/2021) ## 18.0.0 diff --git a/dwds/lib/src/handlers/dev_handler.dart b/dwds/lib/src/handlers/dev_handler.dart index d0526dbc7..c44d12ba7 100644 --- a/dwds/lib/src/handlers/dev_handler.dart +++ b/dwds/lib/src/handlers/dev_handler.dart @@ -28,6 +28,7 @@ import 'package:dwds/src/loaders/strategy.dart'; import 'package:dwds/src/readers/asset_reader.dart'; import 'package:dwds/src/servers/devtools.dart'; import 'package:dwds/src/servers/extension_backend.dart'; +import 'package:dwds/src/servers/extension_debugger.dart'; import 'package:dwds/src/services/app_debug_services.dart'; import 'package:dwds/src/services/debug_service.dart'; import 'package:dwds/src/services/expression_compiler.dart'; @@ -473,109 +474,123 @@ class DevHandler { } Future _listenForDebugExtension() async { - while (await _extensionBackend!.connections.hasNext) { - await _startExtensionDebugService(); + final extensionBackend = _extensionBackend; + if (extensionBackend == null) { + _logger.severe('No debug extension backend. Debugging will not work.'); + return; + } + + while (await extensionBackend.connections.hasNext) { + final extensionDebugger = await extensionBackend.extensionDebugger; + await _startExtensionDebugService(extensionDebugger); } } /// Starts a [DebugService] for Dart Debug Extension. - Future _startExtensionDebugService() async { - final extensionDebugger = await _extensionBackend!.extensionDebugger; + Future _startExtensionDebugService( + ExtensionDebugger extensionDebugger) async { // Waits for a `DevToolsRequest` to be sent from the extension background // when the extension is clicked. extensionDebugger.devToolsRequestStream.listen((devToolsRequest) async { - // TODO(grouma) - Ideally we surface those warnings to the extension so - // that it can be displayed to the user through an alert. - final tabUrl = devToolsRequest.tabUrl; - final appId = devToolsRequest.appId; - if (tabUrl == null) { - _logger.warning('Failed to start extension debug service. ' - 'Missing tab url in DevTools request for app with id: $appId'); - return; - } - final connection = _appConnectionByAppId[appId]; - if (connection == null) { - _logger.warning('Failed to start extension debug service. ' - 'Not connected to an app with id: $appId'); - return; - } - final executionContext = extensionDebugger.executionContext; - if (executionContext == null) { - _logger.warning('Failed to start extension debug service. ' - 'No execution context for app with id: $appId'); - return; + try { + await _handleDevToolsRequest(extensionDebugger, devToolsRequest); + } catch (error) { + _logger.severe('Encountered error handling DevTools request.'); + extensionDebugger.closeWithError(error); } + }); + } - final debuggerStart = DateTime.now(); - var appServices = _servicesByAppId[appId]; - if (appServices == null) { - final debugService = await DebugService.start( - _hostname, - extensionDebugger, - executionContext, - basePathForServerUri(tabUrl), - _assetReader, - _loadStrategy, - connection, - _urlEncoder, - onResponse: (response) { - if (response['error'] == null) return; - _logger - .finest('VmService proxy responded with an error:\n$response'); - }, - useSse: _useSseForDebugProxy, - expressionCompiler: _expressionCompiler, - spawnDds: _spawnDds, - ); - appServices = await _createAppDebugServices( - devToolsRequest.appId, - debugService, - ); - final encodedUri = await debugService.encodedUri; - extensionDebugger.sendEvent('dwds.encodedUri', encodedUri); - safeUnawaited(appServices - .chromeProxyService.remoteDebugger.onClose.first - .whenComplete(() async { - appServices?.chromeProxyService.destroyIsolate(); - await appServices?.close(); - _servicesByAppId.remove(devToolsRequest.appId); - _logger.info('Stopped debug service on ' - '${await appServices?.debugService.encodedUri}\n'); - })); - extensionDebugConnections.add(DebugConnection(appServices)); - _servicesByAppId[appId] = appServices; - } - // If we don't have a DevTools instance, then are connecting to an IDE. - // Therefore return early instead of opening DevTools: - if (_devTools == null) return; - - final encodedUri = await appServices.debugService.encodedUri; - - appServices.dwdsStats.updateLoadTime( - debuggerStart: debuggerStart, devToolsStart: DateTime.now()); - - // TODO(elliette): Remove handling requests from the MV2 extension after - // MV3 release. - // If we only want the URI, this means the Dart Debug Extension should - // handle how to open it. Therefore return early before opening a new - // tab or window: - if (devToolsRequest.uriOnly ?? false) { - final devToolsUri = _constructDevToolsUri( - encodedUri, - ideQueryParam: 'ChromeDevTools', - ); - return extensionDebugger.sendEvent('dwds.devtoolsUri', devToolsUri); - } + Future _handleDevToolsRequest( + ExtensionDebugger extensionDebugger, + DevToolsRequest devToolsRequest, + ) async { + // TODO(grouma) - Ideally we surface those warnings to the extension so + // that it can be displayed to the user through an alert. + final tabUrl = devToolsRequest.tabUrl; + final appId = devToolsRequest.appId; + if (tabUrl == null) { + throw StateError('Failed to start extension debug service. ' + 'Missing tab url in DevTools request for app with id: $appId'); + } + final connection = _appConnectionByAppId[appId]; + if (connection == null) { + throw StateError('Failed to start extension debug service. ' + 'Not connected to an app with id: $appId'); + } + final executionContext = extensionDebugger.executionContext; + if (executionContext == null) { + throw StateError('Failed to start extension debug service. ' + 'No execution context for app with id: $appId'); + } - // Otherwise, launch DevTools in a new tab / window: - await _launchDevTools( + final debuggerStart = DateTime.now(); + var appServices = _servicesByAppId[appId]; + if (appServices == null) { + final debugService = await DebugService.start( + _hostname, extensionDebugger, - _constructDevToolsUri( - encodedUri, - ideQueryParam: 'DebugExtension', - ), + executionContext, + basePathForServerUri(tabUrl), + _assetReader, + _loadStrategy, + connection, + _urlEncoder, + onResponse: (response) { + if (response['error'] == null) return; + _logger.finest('VmService proxy responded with an error:\n$response'); + }, + useSse: _useSseForDebugProxy, + expressionCompiler: _expressionCompiler, + spawnDds: _spawnDds, ); - }); + appServices = await _createAppDebugServices( + devToolsRequest.appId, + debugService, + ); + final encodedUri = await debugService.encodedUri; + extensionDebugger.sendEvent('dwds.encodedUri', encodedUri); + safeUnawaited(appServices.chromeProxyService.remoteDebugger.onClose.first + .whenComplete(() async { + appServices?.chromeProxyService.destroyIsolate(); + await appServices?.close(); + _servicesByAppId.remove(devToolsRequest.appId); + _logger.info('Stopped debug service on ' + '${await appServices?.debugService.encodedUri}\n'); + })); + extensionDebugConnections.add(DebugConnection(appServices)); + _servicesByAppId[appId] = appServices; + } + // If we don't have a DevTools instance, then are connecting to an IDE. + // Therefore return early instead of opening DevTools: + if (_devTools == null) return; + + final encodedUri = await appServices.debugService.encodedUri; + + appServices.dwdsStats.updateLoadTime( + debuggerStart: debuggerStart, devToolsStart: DateTime.now()); + + // TODO(elliette): Remove handling requests from the MV2 extension after + // MV3 release. + // If we only want the URI, this means the Dart Debug Extension should + // handle how to open it. Therefore return early before opening a new + // tab or window: + if (devToolsRequest.uriOnly ?? false) { + final devToolsUri = _constructDevToolsUri( + encodedUri, + ideQueryParam: 'ChromeDevTools', + ); + return extensionDebugger.sendEvent('dwds.devtoolsUri', devToolsUri); + } + + // Otherwise, launch DevTools in a new tab / window: + await _launchDevTools( + extensionDebugger, + _constructDevToolsUri( + encodedUri, + ideQueryParam: 'DebugExtension', + ), + ); } DevTools _ensureDevTools() { diff --git a/dwds/lib/src/servers/extension_debugger.dart b/dwds/lib/src/servers/extension_debugger.dart index f603ba25f..66247d7d0 100644 --- a/dwds/lib/src/servers/extension_debugger.dart +++ b/dwds/lib/src/servers/extension_debugger.dart @@ -13,7 +13,11 @@ import 'package:dwds/src/debugging/execution_context.dart'; import 'package:dwds/src/debugging/remote_debugger.dart'; import 'package:dwds/src/handlers/socket_connections.dart'; import 'package:dwds/src/services/chrome_debug_exception.dart'; -import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; +import 'package:logging/logging.dart'; +import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' + hide StackTrace; + +final _logger = Logger('ExtensionDebugger'); /// A remote debugger backed by the Dart Debug Extension with an SSE connection. class ExtensionDebugger implements RemoteDebugger { @@ -140,11 +144,23 @@ class ExtensionDebugger implements RemoteDebugger { final completer = Completer(); final id = newId(); _completers[id] = completer; - sseConnection.sink - .add(jsonEncode(serializers.serialize(ExtensionRequest((b) => b - ..id = id - ..command = command - ..commandParams = jsonEncode(params ?? {}))))); + try { + sseConnection.sink + .add(jsonEncode(serializers.serialize(ExtensionRequest((b) => b + ..id = id + ..command = command + ..commandParams = jsonEncode(params ?? {}))))); + } on StateError catch (error, stackTrace) { + if (error.message.contains('Cannot add event after closing')) { + _logger.severe('Socket connection closed. Shutting down debugger.'); + closeWithError(error); + } else { + _logger.severe('Bad state while sending $command.', error, stackTrace); + } + } catch (error, stackTrace) { + _logger.severe( + 'Unknown error while sending $command.', error, stackTrace); + } return completer.future; } @@ -161,6 +177,13 @@ class ExtensionDebugger implements RemoteDebugger { ]); }(); + void closeWithError(Object? error) { + _logger.shout( + 'Closing extension debugger due to error. Restart app for debugging functionality', + error); + close(); + } + @override Future disable() => sendCommand('Debugger.disable'); diff --git a/dwds/test/extension_debugger_test.dart b/dwds/test/extension_debugger_test.dart index b02257c85..4e11e1093 100644 --- a/dwds/test/extension_debugger_test.dart +++ b/dwds/test/extension_debugger_test.dart @@ -115,4 +115,38 @@ void main() async { expect(request, extensionRequest); }); }); + group('when closed', () { + test('DebugExtension.detached event closes the connection', () async { + final extensionEvent = ExtensionEvent((b) => b + ..method = jsonEncode('DebugExtension.detached') + ..params = jsonEncode({})); + + connection.controllerIncoming.sink + .add(jsonEncode(serializers.serialize(extensionEvent))); + // Expect the connection to receive a close event: + expect(await extensionDebugger.onClose.first, isNotNull); + }); + + test( + 'gracefully handles trying to send events after the connection is closed', + () async { + // Close the connection: + final extensionEvent = ExtensionEvent((b) => b + ..method = jsonEncode('DebugExtension.detached') + ..params = jsonEncode({})); + connection.controllerIncoming.sink + .add(jsonEncode(serializers.serialize(extensionEvent))); + // Wait for it to be closed: + await extensionDebugger.onClose.first; + // Try to send an event: + callToSendCommand() => extensionDebugger.sendCommand( + 'Debugger.setBreakpoint', + params: { + 'location': {'scriptId': '555', 'lineNumber': 28} + }, + ); + // Should not throw any errors: + expect(callToSendCommand, returnsNormally); + }); + }); }