Skip to content

Fix hot restart hang if injected client throws an error #1670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 15.0.1-dev
- Fix a hang and report errors on hot reload exceptions from the injected
client.

## 15.0.0
- Port some `dwds` files to null safety.
- Fix failing `frontend_server_evaluate` tests.
Expand Down
4 changes: 3 additions & 1 deletion dwds/lib/src/debugging/inspector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,13 @@ class AppInspector extends Domain {
}

/// Evaluate [expression] by calling Chrome's Runtime.evaluate.
Future<RemoteObject> jsEvaluate(String expression) async {
Future<RemoteObject> jsEvaluate(String expression,
{bool awaitPromise = false}) async {
// TODO(alanknight): Support a version with arguments if needed.
WipResponse result;
result = await remoteDebugger.sendCommand('Runtime.evaluate', params: {
'expression': expression,
'awaitPromise': awaitPromise,
'contextId': await contextId,
});
handleErrorIfPresent(result, evalContents: expression, additionalDetails: {
Expand Down
23 changes: 14 additions & 9 deletions dwds/lib/src/dwds_vm_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import 'package:vm_service/vm_service.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';

import 'events.dart';
import 'services/chrome_proxy_service.dart' show ChromeProxyService;
import 'services/chrome_proxy_service.dart';
import 'services/chrome_debug_exception.dart';
import 'services/debug_service.dart';

final _logger = Logger('DwdsVmClient');
Expand Down Expand Up @@ -187,10 +188,9 @@ Future<Map<String, dynamic>> _hotRestart(

chromeProxyService.terminatingIsolates = true;
await _disableBreakpointsAndResume(client, chromeProxyService);
int context;
try {
_logger.info('Attempting to get execution context ID.');
context = await chromeProxyService.executionContext.id;
await chromeProxyService.executionContext.id;
_logger.info('Got execution context ID.');
} on StateError catch (e) {
// We couldn't find the execution context. `hotRestart` may have been
Expand All @@ -209,12 +209,9 @@ Future<Map<String, dynamic>> _hotRestart(
// Generate run id to hot restart all apps loaded into the tab.
final runId = const Uuid().v4().toString();
_logger.info('Issuing \$dartHotRestartDwds request');
await chromeProxyService.remoteDebugger
.sendCommand('Runtime.evaluate', params: {
'expression': '\$dartHotRestartDwds(\'$runId\');',
'awaitPromise': true,
'contextId': context,
});
await chromeProxyService
.appInspectorProvider()
.jsEvaluate('\$dartHotRestartDwds(\'$runId\');', awaitPromise: true);
_logger.info('\$dartHotRestartDwds request complete.');
} on WipError catch (exception) {
final code = exception.error['code'];
Expand All @@ -229,6 +226,14 @@ Future<Map<String, dynamic>> _hotRestart(
}
};
}
} on ChromeDebugException catch (exception) {
// Exceptions thrown by the injected client during hot restart.
return {
'error': {
'code': RPCError.kInternalError,
'message': '$exception',
}
};
}

_logger.info('Waiting for Isolate Start event.');
Expand Down
Loading