Skip to content

Commit 3e86cb8

Browse files
authored
Rename AppRunLogger, stop writing status messages that break JSON (flutter#172591)
Closes flutter#118907. It looks like some of the work around splitting had already happened (`DaemonLogger`), so this ... seems right?
1 parent 465b21c commit 3e86cb8

File tree

6 files changed

+26
-45
lines changed

6 files changed

+26
-45
lines changed

packages/flutter_tools/lib/executable.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ class LoggerFactory {
323323
return NotifyingLogger(verbose: verbose, parent: logger);
324324
}
325325
if (machine) {
326-
return AppRunLogger(parent: logger);
326+
return MachineOutputLogger(parent: logger);
327327
}
328328
return logger;
329329
}

packages/flutter_tools/lib/src/base/logger.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,6 @@ abstract class Logger {
219219
});
220220

221221
/// Send an event to be emitted.
222-
///
223-
/// Only surfaces a value in machine modes, Loggers may ignore this message in
224-
/// non-machine modes.
225222
void sendEvent(String name, [Map<String, dynamic>? args]) {}
226223

227224
/// Clears all output.

packages/flutter_tools/lib/src/commands/attach.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ known, it can be explicitly provided to attach via the command-line, e.g.
378378
true,
379379
_fileSystem.currentDirectory,
380380
LaunchMode.attach,
381-
_logger as AppRunLogger,
381+
_logger as MachineOutputLogger,
382382
);
383383
} on Exception catch (error) {
384384
throwToolExit(error.toString());

packages/flutter_tools/lib/src/commands/daemon.dart

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ class AppDomain extends Domain {
747747
enableHotReload,
748748
cwd,
749749
LaunchMode.run,
750-
asLogger<AppRunLogger>(globals.logger),
750+
asLogger<MachineOutputLogger>(globals.logger),
751751
);
752752
}
753753

@@ -759,7 +759,7 @@ class AppDomain extends Domain {
759759
bool enableHotReload,
760760
Directory cwd,
761761
LaunchMode launchMode,
762-
AppRunLogger logger,
762+
MachineOutputLogger logger,
763763
) async {
764764
final app = AppInstance(
765765
_getNewAppId(),
@@ -771,8 +771,8 @@ class AppDomain extends Domain {
771771

772772
// Set the domain and app for the given AppRunLogger. This allows the logger
773773
// to log messages containing the app ID to the host.
774-
logger.domain = this;
775-
logger.app = app;
774+
logger._domain = this;
775+
logger._app = app;
776776

777777
_sendAppEvent(app, 'start', <String, Object?>{
778778
'deviceId': device.id,
@@ -1549,13 +1549,13 @@ class AppInstance {
15491549
this.id, {
15501550
required this.runner,
15511551
this.logToStdout = false,
1552-
required AppRunLogger logger,
1552+
required MachineOutputLogger logger,
15531553
}) : _logger = logger;
15541554

15551555
final String id;
15561556
final ResidentRunner runner;
15571557
final bool logToStdout;
1558-
final AppRunLogger _logger;
1558+
final MachineOutputLogger _logger;
15591559

15601560
Future<OperationResult> restart({bool fullRestart = false, bool pause = false, String? reason}) {
15611561
return runner.restart(fullRestart: fullRestart, pause: pause, reason: reason);
@@ -1773,21 +1773,13 @@ class ProxyDomain extends Domain {
17731773
..createSync();
17741774
}
17751775

1776-
/// A [Logger] which sends log messages to a listening daemon client.
1777-
///
1778-
/// This class can either:
1779-
/// 1) Send stdout messages and progress events to the client IDE
1780-
/// 1) Log messages to stdout and send progress events to the client IDE
1781-
//
1782-
// TODO(devoncarew): To simplify this code a bit, we could choose to specialize
1783-
// this class into two, one for each of the above use cases.
1784-
class AppRunLogger extends DelegatingLogger {
1785-
AppRunLogger({required Logger parent}) : super(parent);
1786-
1787-
AppDomain? domain;
1788-
late AppInstance app;
1789-
var _nextProgressId = 0;
1776+
/// A [Logger] which omits log messages to avoid breaking `--machine` formatting.
1777+
final class MachineOutputLogger extends DelegatingLogger {
1778+
MachineOutputLogger({required Logger parent}) : super(parent);
17901779

1780+
AppDomain? _domain;
1781+
late final AppInstance _app;
1782+
var _nextProgressId = 0;
17911783
Status? _status;
17921784

17931785
@override
@@ -1814,7 +1806,7 @@ class AppRunLogger extends DelegatingLogger {
18141806
}
18151807

18161808
void close() {
1817-
domain = null;
1809+
_domain = null;
18181810
}
18191811

18201812
void _sendProgressEvent({
@@ -1823,30 +1815,22 @@ class AppRunLogger extends DelegatingLogger {
18231815
bool finished = false,
18241816
String? message,
18251817
}) {
1826-
if (domain == null) {
1827-
// If we're sending progress events before an app has started, send the
1828-
// progress messages as plain status messages.
1829-
if (message != null) {
1830-
printStatus(message);
1831-
}
1832-
} else {
1818+
if (_domain case final domain?) {
18331819
final event = <String, Object?>{
18341820
'id': eventId,
18351821
'progressId': eventType,
18361822
if (message != null) 'message': message,
18371823
'finished': finished,
18381824
};
18391825

1840-
domain!._sendAppEvent(app, 'progress', event);
1826+
domain._sendAppEvent(_app, 'progress', event);
18411827
}
18421828
}
18431829

18441830
@override
18451831
void sendEvent(String name, [Map<String, Object?>? args, List<int>? binary]) {
1846-
if (domain == null) {
1847-
printStatus('event sent after app closed: $name');
1848-
} else {
1849-
domain!.sendEvent(name, args, binary);
1832+
if (_domain case final domain?) {
1833+
domain.sendEvent(name, args, binary);
18501834
}
18511835
}
18521836

packages/flutter_tools/test/commands.shard/hermetic/run_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ void main() {
539539
FileSystem: () => fs,
540540
ProcessManager: () => FakeProcessManager.any(),
541541
Stdio: () => FakeStdio(),
542-
Logger: () => AppRunLogger(parent: logger),
542+
Logger: () => MachineOutputLogger(parent: logger),
543543
},
544544
);
545545

@@ -565,7 +565,7 @@ void main() {
565565
FileSystem: () => fs,
566566
ProcessManager: () => FakeProcessManager.any(),
567567
Stdio: () => FakeStdio(),
568-
Logger: () => AppRunLogger(parent: logger),
568+
Logger: () => MachineOutputLogger(parent: logger),
569569
},
570570
);
571571
});

packages/flutter_tools/test/general.shard/base/logger_test.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ void main() {
9999
daemon: false,
100100
windows: false,
101101
),
102-
isA<AppRunLogger>(),
102+
isA<MachineOutputLogger>(),
103103
);
104104
});
105105

@@ -249,7 +249,7 @@ void main() {
249249
expect(asLogger<VerboseLogger>(notifyingLogger), verboseLogger);
250250
expect(asLogger<FakeLogger>(notifyingLogger), fakeLogger);
251251

252-
expect(() => asLogger<AppRunLogger>(notifyingLogger), throwsStateError);
252+
expect(() => asLogger<MachineOutputLogger>(notifyingLogger), throwsStateError);
253253
});
254254

255255
group('AppContext', () {
@@ -796,13 +796,13 @@ void main() {
796796
expect(lines[3], equals('0123456789' * 3));
797797
});
798798

799-
testWithoutContext('AppRunLogger writes plain text statuses when no app is active', () async {
799+
testWithoutContext('MachineFlagLogger does not output text statuses', () async {
800800
final buffer = BufferLogger.test();
801-
final logger = AppRunLogger(parent: buffer);
801+
final logger = MachineOutputLogger(parent: buffer);
802802

803803
logger.startProgress('Test status...').stop();
804804

805-
expect(buffer.statusText.trim(), equals('Test status...'));
805+
expect(buffer.statusText, isEmpty);
806806
});
807807

808808
testWithoutContext('Error logs are wrapped and can be indented.', () async {

0 commit comments

Comments
 (0)