Unverified Commit ba4d63a4 authored by Jason Simmons's avatar Jason Simmons Committed by GitHub

Exit the tool if a DevTools subprocess fails when running on a bot (#97613)

parent e50ca9ac
...@@ -112,6 +112,7 @@ Future<void> main(List<String> args) async { ...@@ -112,6 +112,7 @@ Future<void> main(List<String> args) async {
processManager: globals.processManager, processManager: globals.processManager,
dartExecutable: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path, dartExecutable: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path,
logger: globals.logger, logger: globals.logger,
botDetector: globals.botDetector,
), ),
Logger: () { Logger: () {
final LoggerFactory loggerFactory = LoggerFactory( final LoggerFactory loggerFactory = LoggerFactory(
......
...@@ -218,6 +218,7 @@ Future<T> runInContext<T>( ...@@ -218,6 +218,7 @@ Future<T> runInContext<T>(
processManager: globals.processManager, processManager: globals.processManager,
dartExecutable: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path, dartExecutable: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path,
logger: globals.logger, logger: globals.logger,
botDetector: globals.botDetector,
), ),
Doctor: () => Doctor(logger: globals.logger), Doctor: () => Doctor(logger: globals.logger),
DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance, DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,
......
...@@ -9,6 +9,8 @@ import 'dart:async'; ...@@ -9,6 +9,8 @@ import 'dart:async';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:process/process.dart'; import 'package:process/process.dart';
import 'base/bot_detector.dart';
import 'base/common.dart';
import 'base/io.dart' as io; import 'base/io.dart' as io;
import 'base/logger.dart'; import 'base/logger.dart';
import 'convert.dart'; import 'convert.dart';
...@@ -21,16 +23,22 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { ...@@ -21,16 +23,22 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
@required ProcessManager processManager, @required ProcessManager processManager,
@required String dartExecutable, @required String dartExecutable,
@required Logger logger, @required Logger logger,
@required BotDetector botDetector,
}) : _processManager = processManager, }) : _processManager = processManager,
_dartExecutable = dartExecutable, _dartExecutable = dartExecutable,
_logger = logger; _logger = logger,
_botDetector = botDetector;
final ProcessManager _processManager; final ProcessManager _processManager;
final String _dartExecutable; final String _dartExecutable;
final Logger _logger; final Logger _logger;
final BotDetector _botDetector;
final Completer<void> _processStartCompleter = Completer<void>(); final Completer<void> _processStartCompleter = Completer<void>();
io.Process _devToolsProcess; io.Process _devToolsProcess;
bool _devToolsProcessKilled = false;
@visibleForTesting
Future<void> devToolsProcessExit;
static final RegExp _serveDevToolsPattern = static final RegExp _serveDevToolsPattern =
RegExp(r'Serving DevTools at ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+?)\.?$'); RegExp(r'Serving DevTools at ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+?)\.?$');
...@@ -66,6 +74,16 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { ...@@ -66,6 +74,16 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
.transform(utf8.decoder) .transform(utf8.decoder)
.transform(const LineSplitter()) .transform(const LineSplitter())
.listen(_logger.printError); .listen(_logger.printError);
final bool runningOnBot = await _botDetector.isRunningOnBot;
devToolsProcessExit = _devToolsProcess.exitCode.then(
(int exitCode) {
if (!_devToolsProcessKilled && runningOnBot) {
throwToolExit('DevTools process failed: exitCode=$exitCode');
}
}
);
devToolsUrl = await completer.future; devToolsUrl = await completer.future;
} on Exception catch (e, st) { } on Exception catch (e, st) {
_logger.printError('Failed to launch DevTools: $e', stackTrace: st); _logger.printError('Failed to launch DevTools: $e', stackTrace: st);
...@@ -86,8 +104,8 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { ...@@ -86,8 +104,8 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
devToolsUrl = null; devToolsUrl = null;
} }
if (_devToolsProcess != null) { if (_devToolsProcess != null) {
_devToolsProcessKilled = true;
_devToolsProcess.kill(); _devToolsProcess.kill();
await _devToolsProcess.exitCode;
} }
} }
} }
...@@ -14,6 +14,7 @@ import 'package:flutter_tools/src/resident_runner.dart'; ...@@ -14,6 +14,7 @@ import 'package:flutter_tools/src/resident_runner.dart';
import '../src/common.dart'; import '../src/common.dart';
import '../src/fake_process_manager.dart'; import '../src/fake_process_manager.dart';
import '../src/fakes.dart';
void main() { void main() {
BufferLogger logger; BufferLogger logger;
...@@ -29,6 +30,7 @@ void main() { ...@@ -29,6 +30,7 @@ void main() {
final DevtoolsLauncher launcher = DevtoolsServerLauncher( final DevtoolsLauncher launcher = DevtoolsServerLauncher(
dartExecutable: 'dart', dartExecutable: 'dart',
logger: logger, logger: logger,
botDetector: const FakeBotDetector(false),
processManager: FakeProcessManager.list(<FakeCommand>[ processManager: FakeProcessManager.list(<FakeCommand>[
FakeCommand( FakeCommand(
command: const <String>[ command: const <String>[
...@@ -52,6 +54,7 @@ void main() { ...@@ -52,6 +54,7 @@ void main() {
final DevtoolsLauncher launcher = DevtoolsServerLauncher( final DevtoolsLauncher launcher = DevtoolsServerLauncher(
dartExecutable: 'dart', dartExecutable: 'dart',
logger: logger, logger: logger,
botDetector: const FakeBotDetector(false),
processManager: FakeProcessManager.list(<FakeCommand>[ processManager: FakeProcessManager.list(<FakeCommand>[
FakeCommand( FakeCommand(
command: const <String>[ command: const <String>[
...@@ -91,6 +94,7 @@ void main() { ...@@ -91,6 +94,7 @@ void main() {
final DevtoolsLauncher launcher = DevtoolsServerLauncher( final DevtoolsLauncher launcher = DevtoolsServerLauncher(
dartExecutable: 'dart', dartExecutable: 'dart',
logger: logger, logger: logger,
botDetector: const FakeBotDetector(false),
processManager: processManager, processManager: processManager,
); );
...@@ -104,6 +108,7 @@ void main() { ...@@ -104,6 +108,7 @@ void main() {
final DevtoolsLauncher launcher = DevtoolsServerLauncher( final DevtoolsLauncher launcher = DevtoolsServerLauncher(
dartExecutable: 'dart', dartExecutable: 'dart',
logger: logger, logger: logger,
botDetector: const FakeBotDetector(false),
processManager: FakeProcessManager.list(<FakeCommand>[ processManager: FakeProcessManager.list(<FakeCommand>[
const FakeCommand( const FakeCommand(
command: <String>[ command: <String>[
...@@ -121,4 +126,29 @@ void main() { ...@@ -121,4 +126,29 @@ void main() {
expect(logger.errorText, contains('Failed to launch DevTools: ProcessException')); expect(logger.errorText, contains('Failed to launch DevTools: ProcessException'));
}); });
testWithoutContext('DevtoolsLauncher handles failure of DevTools process on a bot', () async {
final Completer<void> completer = Completer<void>();
final DevtoolsServerLauncher launcher = DevtoolsServerLauncher(
dartExecutable: 'dart',
logger: logger,
botDetector: const FakeBotDetector(true),
processManager: FakeProcessManager.list(<FakeCommand>[
FakeCommand(
command: const <String>[
'dart',
'devtools',
'--no-launch-browser',
],
stdout: 'Serving DevTools at http://127.0.0.1:9100\n',
completer: completer,
exitCode: 255,
),
]),
);
await launcher.launch(null);
completer.complete();
expect(launcher.devToolsProcessExit, throwsToolExit());
});
} }
...@@ -20,6 +20,7 @@ import 'package:vm_service/vm_service.dart' as vm_service; ...@@ -20,6 +20,7 @@ import 'package:vm_service/vm_service.dart' as vm_service;
import '../src/common.dart'; import '../src/common.dart';
import '../src/fake_process_manager.dart'; import '../src/fake_process_manager.dart';
import '../src/fake_vm_services.dart'; import '../src/fake_vm_services.dart';
import '../src/fakes.dart';
final vm_service.Isolate isolate = vm_service.Isolate( final vm_service.Isolate isolate = vm_service.Isolate(
id: '1', id: '1',
...@@ -106,6 +107,7 @@ void main() { ...@@ -106,6 +107,7 @@ void main() {
processManager: FakeProcessManager.empty(), processManager: FakeProcessManager.empty(),
dartExecutable: 'dart', dartExecutable: 'dart',
logger: BufferLogger.test(), logger: BufferLogger.test(),
botDetector: const FakeBotDetector(false),
); );
final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler( final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
// Uses real devtools instance which should be a no-op if // Uses real devtools instance which should be a no-op if
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment