Unverified Commit 914bd769 authored by Zachary Anderson's avatar Zachary Anderson Committed by GitHub

[flutter_tools] Handle errors on the std{out,err}.done future (#51660)

parent ca9e8b2f
......@@ -42,6 +42,7 @@ import 'dart:io' as io
Stdin,
StdinException,
Stdout,
StdoutException,
stdout;
import 'package:meta/meta.dart';
......@@ -208,16 +209,64 @@ class _PosixProcessSignal extends ProcessSignal {
/// A class that wraps stdout, stderr, and stdin, and exposes the allowed
/// operations.
///
/// In particular, there are three ways that writing to stdout and stderr
/// can fail. A call to stdout.write() can fail:
/// * by throwing a regular synchronous exception,
/// * by throwing an exception asynchronously, and
/// * by completing the Future stdout.done with an error.
///
/// This class enapsulates all three so that we don't have to worry about it
/// anywhere else.
class Stdio {
const Stdio();
Stdio();
/// Tests can provide overrides to use instead of the stdout and stderr from
/// dart:io.
@visibleForTesting
Stdio.test({
@required io.Stdout stdout,
@required io.IOSink stderr,
}) : _stdoutOverride = stdout, _stderrOverride = stderr;
io.Stdout _stdoutOverride;
io.IOSink _stderrOverride;
// These flags exist to remember when the done Futures on stdout and stderr
// complete to avoid trying to write to a closed stream sink, which would
// generate a [StateError].
bool _stdoutDone = false;
bool _stderrDone = false;
Stream<List<int>> get stdin => io.stdin;
@visibleForTesting
io.Stdout get stdout => io.stdout;
io.Stdout get stdout {
if (_stdout != null) {
return _stdout;
}
_stdout = _stdoutOverride ?? io.stdout;
_stdout.done.then(
(void _) { _stdoutDone = true; },
onError: (Object err, StackTrace st) { _stdoutDone = true; },
);
return _stdout;
}
io.Stdout _stdout;
@visibleForTesting
io.IOSink get stderr => io.stderr;
io.IOSink get stderr {
if (_stderr != null) {
return _stderr;
}
_stderr = _stderrOverride ?? io.stderr;
_stderr.done.then(
(void _) { _stderrDone = true; },
onError: (Object err, StackTrace st) { _stderrDone = true; },
);
return _stderr;
}
io.IOSink _stderr;
bool get hasTerminal => io.stdout.hasTerminal;
......@@ -250,25 +299,45 @@ class Stdio {
return _stdinHasTerminal = true;
}
int get terminalColumns => hasTerminal ? io.stdout.terminalColumns : null;
int get terminalLines => hasTerminal ? io.stdout.terminalLines : null;
bool get supportsAnsiEscapes => hasTerminal && io.stdout.supportsAnsiEscapes;
int get terminalColumns => hasTerminal ? stdout.terminalColumns : null;
int get terminalLines => hasTerminal ? stdout.terminalLines : null;
bool get supportsAnsiEscapes => hasTerminal && stdout.supportsAnsiEscapes;
/// Writes [message] to [stderr], falling back on [fallback] if the write
/// throws any exception. The default fallback calls [print] on [message].
void stderrWrite(
String message, {
void Function(String, dynamic, StackTrace) fallback,
}) => _stdioWrite(stderr, message, fallback: fallback);
}) {
if (!_stderrDone) {
_stdioWrite(stderr, message, fallback: fallback);
return;
}
fallback == null ? print(message) : fallback(
message,
const io.StdoutException('stderr is done'),
StackTrace.current,
);
}
/// Writes [message] to [stdout], falling back on [fallback] if the write
/// throws any exception. The default fallback calls [print] on [message].
void stdoutWrite(
String message, {
void Function(String, dynamic, StackTrace) fallback,
}) => _stdioWrite(stdout, message, fallback: fallback);
}) {
if (!_stdoutDone) {
_stdioWrite(stdout, message, fallback: fallback);
return;
}
fallback == null ? print(message) : fallback(
message,
const io.StdoutException('stdout is done'),
StackTrace.current,
);
}
// Helper for safeStderrWrite and safeStdoutWrite.
// Helper for [stderrWrite] and [stdoutWrite].
void _stdioWrite(io.IOSink sink, String message, {
void Function(String, dynamic, StackTrace) fallback,
}) {
......
......@@ -175,7 +175,7 @@ Future<T> runInContext<T>(
ShutdownHooks: () => ShutdownHooks(logger: globals.logger),
Signals: () => Signals(),
SimControl: () => SimControl(),
Stdio: () => const Stdio(),
Stdio: () => Stdio(),
SystemClock: () => const SystemClock(),
TimeoutConfiguration: () => const TimeoutConfiguration(),
Usage: () => Usage(
......
......@@ -150,14 +150,16 @@ final AnsiTerminal _defaultAnsiTerminal = AnsiTerminal(
);
/// The global Stdio wrapper.
Stdio get stdio => context.get<Stdio>() ?? const Stdio();
Stdio get stdio => context.get<Stdio>() ?? (_stdioInstance ??= Stdio());
Stdio _stdioInstance;
PlistParser get plistParser => context.get<PlistParser>() ?? (_defaultInstance ??= PlistParser(
PlistParser get plistParser => context.get<PlistParser>() ?? (
_plistInstance ??= PlistParser(
fileSystem: fs,
processManager: processManager,
logger: logger,
));
PlistParser _defaultInstance;
PlistParser _plistInstance;
/// The [ChromeLauncher] instance.
ChromeLauncher get chromeLauncher => context.get<ChromeLauncher>();
......
......@@ -23,16 +23,6 @@ final String resetColor = RegExp.escape(AnsiTerminal.resetColor);
class MockStdout extends Mock implements Stdout {}
class ThrowingStdio extends Stdio {
ThrowingStdio(this.stdout, this.stderr);
@override
final Stdout stdout;
@override
final IOSink stderr;
}
void main() {
group('AppContext', () {
FakeStopwatch fakeStopWatch;
......@@ -94,9 +84,11 @@ void main() {
testWithoutContext('Logger does not throw when stdio write throws synchronously', () async {
final MockStdout stdout = MockStdout();
final MockStdout stderr = MockStdout();
final ThrowingStdio stdio = ThrowingStdio(stdout, stderr);
final Stdio stdio = Stdio.test(stdout: stdout, stderr: stderr);
bool stdoutThrew = false;
bool stderrThrew = false;
final Completer<void> stdoutError = Completer<void>();
final Completer<void> stderrError = Completer<void>();
when(stdout.write(any)).thenAnswer((_) {
stdoutThrew = true;
throw 'Error';
......@@ -105,6 +97,8 @@ void main() {
stderrThrew = true;
throw 'Error';
});
when(stdout.done).thenAnswer((_) => stdoutError.future);
when(stderr.done).thenAnswer((_) => stderrError.future);
final Logger logger = StdoutLogger(
terminal: AnsiTerminal(
stdio: stdio,
......@@ -123,7 +117,9 @@ void main() {
testWithoutContext('Logger does not throw when stdio write throws asynchronously', () async {
final MockStdout stdout = MockStdout();
final MockStdout stderr = MockStdout();
final ThrowingStdio stdio = ThrowingStdio(stdout, stderr);
final Stdio stdio = Stdio.test(stdout: stdout, stderr: stderr);
final Completer<void> stdoutError = Completer<void>();
final Completer<void> stderrError = Completer<void>();
bool stdoutThrew = false;
bool stderrThrew = false;
final Completer<void> stdoutCompleter = Completer<void>();
......@@ -142,6 +138,8 @@ void main() {
throw 'Error';
}, null);
});
when(stdout.done).thenAnswer((_) => stdoutError.future);
when(stderr.done).thenAnswer((_) => stderrError.future);
final Logger logger = StdoutLogger(
terminal: AnsiTerminal(
stdio: stdio,
......@@ -159,6 +157,43 @@ void main() {
expect(stderrThrew, true);
});
testWithoutContext('Logger does not throw when stdio completes done with an error', () async {
final MockStdout stdout = MockStdout();
final MockStdout stderr = MockStdout();
final Stdio stdio = Stdio.test(stdout: stdout, stderr: stderr);
final Completer<void> stdoutError = Completer<void>();
final Completer<void> stderrError = Completer<void>();
final Completer<void> stdoutCompleter = Completer<void>();
final Completer<void> stderrCompleter = Completer<void>();
when(stdout.write(any)).thenAnswer((_) {
Zone.current.runUnaryGuarded<void>((_) {
stdoutError.completeError(Exception('Some pipe error'));
stdoutCompleter.complete();
}, null);
});
when(stderr.write(any)).thenAnswer((_) {
Zone.current.runUnaryGuarded<void>((_) {
stderrError.completeError(Exception('Some pipe error'));
stderrCompleter.complete();
}, null);
});
when(stdout.done).thenAnswer((_) => stdoutError.future);
when(stderr.done).thenAnswer((_) => stderrError.future);
final Logger logger = StdoutLogger(
terminal: AnsiTerminal(
stdio: stdio,
platform: _kNoAnsiPlatform,
),
stdio: stdio,
outputPreferences: OutputPreferences.test(),
timeoutConfiguration: const TimeoutConfiguration(),
);
logger.printStatus('message');
logger.printError('error message');
await stdoutCompleter.future;
await stderrCompleter.future;
});
group('Spinners', () {
mocks.MockStdio mockStdio;
FakeStopwatch mockStopwatch;
......
......@@ -56,7 +56,7 @@ void main() {
await verifyCrashReportSent(requestInfo);
}, overrides: <Type, Generator>{
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
testUsingContext('should print an explanatory message when there is a SocketException', () async {
......@@ -77,7 +77,7 @@ void main() {
expect(await exitCodeCompleter.future, 1);
expect(testLogger.errorText, contains('Failed to send crash report due to a network error'));
}, overrides: <Type, Generator>{
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
testUsingContext('should print an explanatory message when there is an HttpException', () async {
......@@ -98,7 +98,7 @@ void main() {
expect(await exitCodeCompleter.future, 1);
expect(testLogger.errorText, contains('Failed to send crash report due to a network error'));
}, overrides: <Type, Generator>{
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
testUsingContext('should send crash reports when async throws', () async {
......@@ -120,7 +120,7 @@ void main() {
expect(await exitCodeCompleter.future, 1);
await verifyCrashReportSent(requestInfo);
}, overrides: <Type, Generator>{
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
testUsingContext('should send only one crash report when async throws many', () async {
......@@ -151,7 +151,7 @@ void main() {
await verifyCrashReportSent(requestInfo, crashes: 4);
}, overrides: <Type, Generator>{
DoctorValidatorsProvider: () => FakeDoctorValidatorsProvider(),
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
testUsingContext('should not send a crash report if on a user-branch', () async {
......@@ -183,7 +183,7 @@ void main() {
expect(testLogger.traceText, isNot(contains('Crash report sent')));
}, overrides: <Type, Generator>{
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
testUsingContext('can override base URL', () async {
......@@ -223,7 +223,7 @@ void main() {
},
script: Uri(scheme: 'data'),
),
Stdio: () => const _NoStderr(),
Stdio: () => _NoStderr(),
});
});
}
......@@ -400,7 +400,7 @@ class FakeDoctorValidatorsProvider implements DoctorValidatorsProvider {
}
class _NoStderr extends Stdio {
const _NoStderr();
_NoStderr();
@override
IOSink get stderr => const _NoopIOSink();
......
......@@ -16,12 +16,13 @@ const String _kInitialVersion = 'v1.9.1+hotfix.6';
const String _kBranch = 'stable';
const FileSystem fileSystem = LocalFileSystem();
const ProcessManager processManager = LocalProcessManager();
final Stdio stdio = Stdio();
final ProcessUtils processUtils = ProcessUtils(processManager: processManager, logger: StdoutLogger(
terminal: AnsiTerminal(
platform: const LocalPlatform(),
stdio: const Stdio(),
stdio: stdio,
),
stdio: const Stdio(),
stdio: stdio,
outputPreferences: OutputPreferences.test(wrapText: true),
timeoutConfiguration: const TimeoutConfiguration(),
));
......
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