diff --git a/engine/src/flutter/lib/web_ui/dev/pipeline.dart b/engine/src/flutter/lib/web_ui/dev/pipeline.dart index 54cb5aa3b5b..471b48bcf8e 100644 --- a/engine/src/flutter/lib/web_ui/dev/pipeline.dart +++ b/engine/src/flutter/lib/web_ui/dev/pipeline.dart @@ -89,10 +89,11 @@ abstract class ProcessStep implements PipelineStep { } class _PipelineStepFailure { - _PipelineStepFailure(this.step, this.error); + _PipelineStepFailure(this.step, this.error, this.stackTrace); final PipelineStep step; final Object error; + final StackTrace stackTrace; } /// Executes a sequence of asynchronous tasks, typically as part of a build/test @@ -133,8 +134,8 @@ class Pipeline { _currentStepFuture = step.run(); try { await _currentStepFuture; - } catch (e) { - failures.add(_PipelineStepFailure(step, e)); + } catch (error, stackTrace) { + failures.add(_PipelineStepFailure(step, error, stackTrace)); } finally { _currentStep = null; } @@ -145,7 +146,7 @@ class Pipeline { _status = PipelineStatus.error; print('Pipeline experienced the following failures:'); for (final _PipelineStepFailure failure in failures) { - print(' "${failure.step.description}": ${failure.error}'); + print(' "${failure.step.description}": ${failure.error}\n${failure.stackTrace}'); } throw ToolExit('Test pipeline failed.'); } diff --git a/engine/src/flutter/lib/web_ui/dev/safari_macos.dart b/engine/src/flutter/lib/web_ui/dev/safari_macos.dart index a7405f54b70..73d0e6c86c8 100644 --- a/engine/src/flutter/lib/web_ui/dev/safari_macos.dart +++ b/engine/src/flutter/lib/web_ui/dev/safari_macos.dart @@ -7,11 +7,21 @@ import 'dart:convert'; import 'dart:io'; import 'package:test_api/backend.dart'; +import 'package:webdriver/async_io.dart' show WebDriver, createDriver; +import 'browser.dart'; import 'webdriver_browser.dart'; /// Provides an environment for the desktop variant of Safari running on macOS. -class SafariMacOsEnvironment extends WebDriverBrowserEnvironment { +class SafariMacOsEnvironment extends BrowserEnvironment { + static const Duration _waitBetweenRetries = Duration(seconds: 1); + static const int _maxRetryCount = 5; + + late int _portNumber; + late Process _driverProcess; + Uri get _driverUri => Uri(scheme: 'http', host: 'localhost', port: _portNumber); + WebDriver? webDriver; + @override final String name = 'Safari macOS'; @@ -21,21 +31,34 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment { @override String get packageTestConfigurationYamlFile => 'dart_test_safari.yaml'; - @override - Uri get driverUri => Uri(scheme: 'http', host: 'localhost', port: portNumber); - - late Process _driverProcess; - int _retryCount = 0; - static const int _waitBetweenRetryInSeconds = 1; - static const int _maxRetryCount = 10; - - @override - Future spawnDriverProcess() => - Process.start('safaridriver', ['-p', portNumber.toString()]); - @override Future prepare() async { - await _startDriverProcess(); + int retryCount = 0; + + while (true) { + try { + if (retryCount > 0) { + print('Retry #$retryCount'); + } + retryCount += 1; + await _startDriverProcess(); + return; + } catch (error, stackTrace) { + if (retryCount < _maxRetryCount) { + print(''' +Failed to start safaridriver: + +Error: $error +$stackTrace +'''); + print('Will try again.'); + await Future.delayed(_waitBetweenRetries); + } else { + print('Too many retries. Giving up.'); + rethrow; + } + } + } } /// Pick an unused port and start `safaridriver` using that port. @@ -45,36 +68,130 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment { /// again with a different port. Wait [_waitBetweenRetryInSeconds] seconds /// between retries. Try up to [_maxRetryCount] times. Future _startDriverProcess() async { - _retryCount += 1; - if (_retryCount > 1) { - await Future.delayed(const Duration(seconds: _waitBetweenRetryInSeconds)); - } - portNumber = await pickUnusedPort(); + _portNumber = await pickUnusedPort(); + print('Starting safaridriver on port $_portNumber'); - print('Attempt $_retryCount to start safaridriver on port $portNumber'); + try { + _driverProcess = await Process.start('safaridriver', ['-p', _portNumber.toString()]); - _driverProcess = await spawnDriverProcess(); + _driverProcess.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen(( + String log, + ) { + print('[safaridriver] $log'); + }); - _driverProcess.stderr.transform(utf8.decoder).transform(const LineSplitter()).listen(( - String error, - ) { - print('[Webdriver][Error] $error'); - if (_retryCount > _maxRetryCount) { - print('[Webdriver][Error] Failed to start after $_maxRetryCount tries.'); - } else if (error.contains('Operation not permitted')) { - _driverProcess.kill(); - _startDriverProcess(); + _driverProcess.stderr.transform(utf8.decoder).transform(const LineSplitter()).listen(( + String error, + ) { + print('[safaridriver][error] $error'); + }); + + await _waitForSafariDriverServerReady(); + + // Smoke-test the web driver process by connecting to it and asking for a + // list of windows. It doesn't matter how many windows there are. + webDriver = await createDriver( + uri: _driverUri, + desired: {'browserName': packageTestRuntime.identifier}, + ); + + await webDriver!.windows.toList(); + } catch (_) { + print('safaridriver failed to start.'); + + final badDriver = webDriver; + webDriver = null; // let's not keep faulty driver around + + if (badDriver != null) { + // This means the launch process got to a point where a WebDriver + // instance was created, but it failed the smoke test. To make sure no + // stray driver sessions are left hanging, try to close the session. + try { + // The method is called "quit" but all it does is close the session. + // + // See: https://www.w3.org/TR/webdriver2/#delete-session + await badDriver.quit(); + } catch (error, stackTrace) { + // Just print. Do not rethrow. The attempt to close the session is + // only a best-effort thing. + print(''' +Failed to close driver session. Will try to kill the safaridriver process. + +Error: $error +$stackTrace +'''); + } } - }); - _driverProcess.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen(( - String log, - ) { - print('[Webdriver] $log'); - }); + + // Try to kill gracefully using SIGTERM first. + _driverProcess.kill(); + await _driverProcess.exitCode.timeout( + const Duration(seconds: 2), + onTimeout: () async { + // If the process fails to exit gracefully in a reasonable amount of + // time, kill it forcefully. + print('safaridriver failed to exit normally. Killing with SIGKILL.'); + _driverProcess.kill(ProcessSignal.sigkill); + return 0; + }, + ); + + // Rethrow the error to allow the caller to retry, if need be. + rethrow; + } + } + + /// The Safari Driver process cannot instantly spawn a server, so this function + /// attempts to connect to the server in a loop until it succeeds. + /// + /// A healthy driver process is expected to respond to a `GET /status` HTTP + /// request with `{value: {ready: true}}` JSON response. + /// + /// See also: https://www.w3.org/TR/webdriver2/#status + Future _waitForSafariDriverServerReady() async { + // Wait just a tiny bit before connecting for the very first time because + // frequently safaridriver isn't quick enough to bring up the server. + // + // 100ms seems enough in most cases, but feel free to revisit this. + await Future.delayed(const Duration(milliseconds: 100)); + + int retryCount = 0; + while (true) { + retryCount += 1; + final httpClient = HttpClient(); + try { + final request = await httpClient.get('localhost', _portNumber, '/status'); + final response = await request.close(); + final stringData = await response.transform(utf8.decoder).join(); + final jsonResponse = json.decode(stringData) as Map; + final value = jsonResponse['value']! as Map; + final ready = value['ready']! as bool; + if (ready) { + break; + } + } catch (_) { + if (retryCount < 10) { + print('safaridriver not ready yet. Waiting...'); + await Future.delayed(const Duration(milliseconds: 100)); + } else { + print( + 'safaridriver failed to reach ready state in a reasonable amount of time. Giving up.', + ); + rethrow; + } + } + } + } + + @override + Future launchBrowserInstance(Uri url, {bool debug = false}) async { + return WebDriverBrowser(webDriver!, url); } @override Future cleanup() async { + // WebDriver.quit() is not called here, because that's done in + // WebDriverBrowser.close(). _driverProcess.kill(); } } diff --git a/engine/src/flutter/lib/web_ui/dev/webdriver_browser.dart b/engine/src/flutter/lib/web_ui/dev/webdriver_browser.dart index efe0e023f6f..ab4d32e3314 100644 --- a/engine/src/flutter/lib/web_ui/dev/webdriver_browser.dart +++ b/engine/src/flutter/lib/web_ui/dev/webdriver_browser.dart @@ -3,80 +3,25 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:convert'; import 'dart:io'; import 'dart:math'; import 'package:image/image.dart'; -import 'package:webdriver/async_io.dart' show WebDriver, createDriver; +import 'package:webdriver/async_io.dart' show WebDriver; import 'browser.dart'; -abstract class WebDriverBrowserEnvironment extends BrowserEnvironment { - late int portNumber; - late final Process _driverProcess; - - Future spawnDriverProcess(); - Uri get driverUri; - - /// Finds and returns an unused port on the test host in the local port range. - Future pickUnusedPort() async { - // Use bind to allocate an unused port, then unbind from that port to - // make it available for use. - final ServerSocket socket = await ServerSocket.bind('localhost', 0); - final int port = socket.port; - await socket.close(); - - return port; - } - - @override - Future prepare() async { - portNumber = await pickUnusedPort(); - - _driverProcess = await spawnDriverProcess(); - - _driverProcess.stderr.transform(utf8.decoder).transform(const LineSplitter()).listen(( - String error, - ) { - print('[Webdriver][Error] $error'); - }); - - _driverProcess.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen(( - String log, - ) { - print('[Webdriver] $log'); - }); - } - - @override - Future cleanup() async { - _driverProcess.kill(); - } - - @override - Future launchBrowserInstance(Uri url, {bool debug = false}) async { - while (true) { - try { - final WebDriver driver = await createDriver( - uri: driverUri, - desired: {'browserName': packageTestRuntime.identifier}, - ); - return WebDriverBrowser(driver, url); - } on SocketException { - // Sometimes we may try to connect before the web driver port is ready. - // So we should retry here. Note that if there was some issue with the - // webdriver process, we may loop infinitely here, so we're relying on - // the test timeout to kill us if it takes too long to connect. - print('Failed to connect to webdriver process. Retrying in 100 ms'); - await Future.delayed(const Duration(milliseconds: 100)); - } catch (exception) { - rethrow; - } - } - } +/// Finds and returns an unused port on the test host in the local port range. +Future pickUnusedPort() async { + // Use bind to allocate an unused port, then unbind from that port to + // make it available for use. + final ServerSocket socket = await ServerSocket.bind('localhost', 0); + final int port = socket.port; + await socket.close(); + return port; } +/// A browser managed through a WebDriver connection. class WebDriverBrowser extends Browser { WebDriverBrowser(this._driver, this._url) { _driver.get(_url); @@ -104,7 +49,7 @@ class WebDriverBrowser extends Browser { _shouldStopActivating = true; await _activateLoopFuture; - await (await _driver.window).close(); + await _driver.quit(); if (!_onExitCompleter.isCompleted) { _onExitCompleter.complete(); }