Unverified Commit 2b635816 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Wrap dwds in async guard, only catch known error types (#42364)

parent 03a45dc3
...@@ -49,11 +49,11 @@ import 'dart:async'; ...@@ -49,11 +49,11 @@ import 'dart:async';
/// ///
/// As such, it would be convenient if the `try {} catch {}` here could handle /// As such, it would be convenient if the `try {} catch {}` here could handle
/// not only errors completing the awaited [Future]s it contains, but also /// not only errors completing the awaited [Future]s it contains, but also
/// any otherwise unhandled asynchronous errors occuring as a result of awaited /// any otherwise unhandled asynchronous errors occurring as a result of awaited
/// expressions. This is how `await` is often assumed to work, which leads to /// expressions. This is how `await` is often assumed to work, which leads to
/// unexpected unhandled exceptions. /// unexpected unhandled exceptions.
/// ///
/// [asyncGuard] is intended to wrap awaited expressions occuring in a `try` /// [asyncGuard] is intended to wrap awaited expressions occurring in a `try`
/// block. The behavior described above gives the behavior that users /// block. The behavior described above gives the behavior that users
/// intuitively expect from `await`. Consider the snippet: /// intuitively expect from `await`. Consider the snippet:
/// ``` /// ```
......
...@@ -6,11 +6,13 @@ import 'dart:async'; ...@@ -6,11 +6,13 @@ import 'dart:async';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:vm_service/vm_service.dart' as vmservice; import 'package:vm_service/vm_service.dart' as vmservice;
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' hide StackTrace;
import '../application_package.dart'; import '../application_package.dart';
import '../base/async_guard.dart';
import '../base/common.dart'; import '../base/common.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/io.dart';
import '../base/logger.dart'; import '../base/logger.dart';
import '../base/terminal.dart'; import '../base/terminal.dart';
import '../base/utils.dart'; import '../base/utils.dart';
...@@ -164,46 +166,57 @@ class ResidentWebRunner extends ResidentRunner { ...@@ -164,46 +166,57 @@ class ResidentWebRunner extends ResidentRunner {
printStatus('Launching ${getDisplayPath(target)} on ${device.name} in $modeName mode...'); printStatus('Launching ${getDisplayPath(target)} on ${device.name} in $modeName mode...');
Status buildStatus; Status buildStatus;
try { try {
// dwds does not handle uncaught exceptions from its servers. To work
// around this, we need to catch all uncaught exceptions and determine if
// they are fatal or not.
buildStatus = logger.startProgress('Building application for the web...', timeout: null); buildStatus = logger.startProgress('Building application for the web...', timeout: null);
_webFs = await webFsFactory( final int result = await asyncGuard(() async {
target: target, _webFs = await webFsFactory(
flutterProject: flutterProject, target: target,
buildInfo: debuggingOptions.buildInfo, flutterProject: flutterProject,
initializePlatform: debuggingOptions.initializePlatform, buildInfo: debuggingOptions.buildInfo,
hostname: debuggingOptions.hostname, initializePlatform: debuggingOptions.initializePlatform,
port: debuggingOptions.port, hostname: debuggingOptions.hostname,
skipDwds: device is WebServerDevice || !debuggingOptions.buildInfo.isDebug, port: debuggingOptions.port,
); skipDwds: device is WebServerDevice || !debuggingOptions.buildInfo.isDebug,
// When connecting to a browser, update the message with a seemsSlow notification
// to handle the case where we fail to connect.
if (debuggingOptions.browserLaunch) {
buildStatus.stop();
buildStatus = logger.startProgress(
'Attempting to connect to browser instance..',
timeout: const Duration(seconds: 30),
); );
} // When connecting to a browser, update the message with a seemsSlow notification
await device.startApp(package, // to handle the case where we fail to connect.
mainPath: target, if (debuggingOptions.browserLaunch) {
debuggingOptions: debuggingOptions, buildStatus.stop();
platformArgs: <String, Object>{ buildStatus = logger.startProgress(
'uri': _webFs.uri, 'Attempting to connect to browser instance..',
}, timeout: const Duration(seconds: 30),
); );
if (supportsServiceProtocol) { }
_connectionResult = await _webFs.connect(debuggingOptions); await device.startApp(package,
unawaited(_connectionResult.debugConnection.onDone.whenComplete(exit)); mainPath: target,
} debuggingOptions: debuggingOptions,
} catch (err) { platformArgs: <String, Object>{
throwToolExit('Failed to build application for the web.'); 'uri': _webFs.uri,
},
);
if (supportsServiceProtocol) {
_connectionResult = await _webFs.connect(debuggingOptions);
unawaited(_connectionResult.debugConnection.onDone.whenComplete(() => exit(0)));
}
appStartedCompleter?.complete();
return attach(
connectionInfoCompleter: connectionInfoCompleter,
appStartedCompleter: appStartedCompleter,
);
});
return result;
} on WebSocketException {
throwToolExit('Failed to connect to WebSocket.');
} on BuildException {
throwToolExit('Failed to build application for the Web.');
} on SocketException catch (err) {
throwToolExit(err.toString());
} finally { } finally {
buildStatus.stop(); buildStatus.stop();
} }
appStartedCompleter?.complete(); return 1;
return attach(
connectionInfoCompleter: connectionInfoCompleter,
appStartedCompleter: appStartedCompleter,
);
} }
@override @override
......
...@@ -324,13 +324,21 @@ class WebFs { ...@@ -324,13 +324,21 @@ class WebFs {
initializePlatform, initializePlatform,
); );
if (!await firstBuildCompleter.future) { if (!await firstBuildCompleter.future) {
throw Exception('Failed to compile for the web.'); throw const BuildException();
} }
await firstBuild?.cancel(); await firstBuild?.cancel();
return webFS; return webFS;
} }
} }
/// An exception thrown when build runner fails.
///
/// This contains no error information as it will have already been printed to
/// the console.
class BuildException implements Exception {
const BuildException();
}
abstract class AssetServer { abstract class AssetServer {
Future<Response> handle(Request request); Future<Response> handle(Request request);
......
...@@ -8,6 +8,7 @@ import 'dart:convert'; ...@@ -8,6 +8,7 @@ import 'dart:convert';
import 'package:dwds/dwds.dart'; import 'package:dwds/dwds.dart';
import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/device.dart';
...@@ -459,6 +460,63 @@ void main() { ...@@ -459,6 +460,63 @@ void main() {
expect(bufferLogger.statusText, contains('Launching ${fs.path.join('lib', 'main.dart')} on Chromez in debug mode')); expect(bufferLogger.statusText, contains('Launching ${fs.path.join('lib', 'main.dart')} on Chromez in debug mode'));
})); }));
test('Successfully turns WebSocketException into ToolExit', () => testbed.run(() async {
_setupMocks();
final Completer<DebugConnectionInfo> connectionInfoCompleter = Completer<DebugConnectionInfo>();
final Completer<void> unhandledErrorCompleter = Completer<void>();
when(mockWebFs.connect(any)).thenAnswer((Invocation _) async {
unawaited(unhandledErrorCompleter.future.then((void value) {
throw const WebSocketException();
}));
return ConnectionResult(mockAppConnection, mockDebugConnection);
});
final Future<void> expectation = expectLater(() => residentWebRunner.run(
connectionInfoCompleter: connectionInfoCompleter,
), throwsA(isInstanceOf<ToolExit>()));
unhandledErrorCompleter.complete();
await expectation;
}));
test('Rethrows Exception type', () => testbed.run(() async {
_setupMocks();
final Completer<DebugConnectionInfo> connectionInfoCompleter = Completer<DebugConnectionInfo>();
final Completer<void> unhandledErrorCompleter = Completer<void>();
when(mockWebFs.connect(any)).thenAnswer((Invocation _) async {
unawaited(unhandledErrorCompleter.future.then((void value) {
throw Exception('Something went wrong');
}));
return ConnectionResult(mockAppConnection, mockDebugConnection);
});
final Future<void> expectation = expectLater(() => residentWebRunner.run(
connectionInfoCompleter: connectionInfoCompleter,
), throwsA(isInstanceOf<Exception>()));
unhandledErrorCompleter.complete();
await expectation;
}));
test('Rethrows unknown exception type from web tooling', () => testbed.run(() async {
_setupMocks();
final Completer<DebugConnectionInfo> connectionInfoCompleter = Completer<DebugConnectionInfo>();
final Completer<void> unhandledErrorCompleter = Completer<void>();
when(mockWebFs.connect(any)).thenAnswer((Invocation _) async {
unawaited(unhandledErrorCompleter.future.then((void value) {
throw StateError('Something went wrong');
}));
return ConnectionResult(mockAppConnection, mockDebugConnection);
});
final Future<void> expectation = expectLater(() => residentWebRunner.run(
connectionInfoCompleter: connectionInfoCompleter,
), throwsA(isInstanceOf<StateError>()));
unhandledErrorCompleter.complete();
await expectation;
}));
} }
class MockFlutterUsage extends Mock implements Usage {} class MockFlutterUsage extends Mock implements Usage {}
......
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