Skip to content

Commit bf082fe

Browse files
authored
ci(shorebird): stream child stdout/stderr from shard_runner (#144)
* ci(shorebird): stream child stdout/stderr from shard_runner `runChecked` used `Process.run`, which buffers stdout/stderr in memory and prints nothing until the process exits — and on success, prints nothing at all. That hid 30+ minutes of ninja, cargo, and gclient output per shard from the GitHub Actions log; only the [Step] start / [Step] complete prints from the runner itself made it through. The legacy bash build scripts streamed naturally, so this is a regression the sharded runner introduced. Switch `runChecked` to `Process.start` with inheritStdio so output goes live to the parent's stdio. The error path no longer needs to echo captured output (it already streamed) — the exception just notes the exit code and description. Add `runCapturingStdout` for the one place that actually parses child output (`gsutil ls` in lib/gcs.dart). It captures stdout while still streaming stderr, so failures still appear in the log live. Tests: 32/32 pass; dart analyze clean. * fix: dart format gcs.dart
1 parent c063af1 commit bf082fe

2 files changed

Lines changed: 59 additions & 23 deletions

File tree

shorebird/ci/shard_runner/lib/gcs.dart

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,16 @@ Future<void> downloadFromStaging({
7272

7373
print('[GCS] Downloading from $stagingRoot');
7474

75-
// List files in the staging location
76-
final ProcessResult lsResult = await runChecked(
75+
// List files in the staging location. We capture stdout because we need
76+
// to parse the file list; everything else uses runChecked to stream.
77+
final String lsOutput = await runCapturingStdout(
7778
'gsutil',
7879
<String>['ls', stagingRoot],
7980
description: 'gsutil ls $stagingRoot',
8081
);
8182

82-
final List<String> files = (lsResult.stdout as String)
83-
.split('\n')
84-
.where((String f) => f.endsWith('.tar.gz'))
85-
.toList();
83+
final List<String> files =
84+
lsOutput.split('\n').where((String f) => f.endsWith('.tar.gz')).toList();
8685

8786
for (final String file in files) {
8887
final String fileName = p.basename(file);

shorebird/ci/shard_runner/lib/process.dart

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:async';
2+
import 'dart:convert';
13
import 'dart:io';
24

35
/// Resolves an executable name for Windows, appending .cmd if needed.
@@ -28,10 +30,13 @@ String _resolveExecutable(String executable) {
2830
return executable;
2931
}
3032

31-
/// Runs a process and throws if it exits with a non-zero code.
33+
/// Runs a process, streaming stdout/stderr to the parent's stdio so the
34+
/// caller (and CI logs) see output live. Throws if the process exits
35+
/// non-zero.
3236
///
33-
/// Returns the [ProcessResult] for callers that need stdout/stderr.
34-
Future<ProcessResult> runChecked(
37+
/// Use this for everything except the rare case where you need to parse
38+
/// the child's stdout — for that, see [runCapturingStdout].
39+
Future<void> runChecked(
3540
String executable,
3641
List<String> arguments, {
3742
String? workingDirectory,
@@ -40,27 +45,59 @@ Future<ProcessResult> runChecked(
4045
}) async {
4146
final String resolvedExecutable = _resolveExecutable(executable);
4247

43-
final ProcessResult result = await Process.run(
48+
final Process process = await Process.start(
4449
resolvedExecutable,
4550
arguments,
4651
workingDirectory: workingDirectory,
4752
environment: environment,
53+
mode: ProcessStartMode.inheritStdio,
4854
);
4955

50-
if (result.exitCode != 0) {
56+
final int exitCode = await process.exitCode;
57+
if (exitCode != 0) {
5158
final String desc = description ?? '$executable ${arguments.join(' ')}';
52-
final String stderr = (result.stderr as String).trim();
53-
final String stdout = (result.stdout as String).trim();
54-
final StringBuffer message =
55-
StringBuffer('$desc failed (exit ${result.exitCode})');
56-
if (stdout.isNotEmpty) {
57-
message.write('\nSTDOUT: $stdout');
58-
}
59-
if (stderr.isNotEmpty) {
60-
message.write('\nSTDERR: $stderr');
61-
}
62-
throw Exception(message.toString());
59+
throw Exception('$desc failed (exit $exitCode)');
6360
}
61+
}
62+
63+
/// Runs a process, capturing stdout into the returned string while still
64+
/// streaming stderr to the parent's stderr. Throws if the process exits
65+
/// non-zero.
66+
///
67+
/// Use this when you need to parse the child's stdout (e.g. `gsutil ls`).
68+
/// For everything else use [runChecked] so output reaches the CI log live.
69+
Future<String> runCapturingStdout(
70+
String executable,
71+
List<String> arguments, {
72+
String? workingDirectory,
73+
Map<String, String>? environment,
74+
String? description,
75+
}) async {
76+
final String resolvedExecutable = _resolveExecutable(executable);
6477

65-
return result;
78+
final Process process = await Process.start(
79+
resolvedExecutable,
80+
arguments,
81+
workingDirectory: workingDirectory,
82+
environment: environment,
83+
);
84+
85+
final Future<String> stdoutFuture =
86+
process.stdout.transform(utf8.decoder).join();
87+
final Future<void> stderrFuture =
88+
process.stderr.transform(utf8.decoder).forEach(stderr.write);
89+
90+
final List<Object?> results = await Future.wait<Object?>(<Future<Object?>>[
91+
stdoutFuture,
92+
stderrFuture,
93+
process.exitCode,
94+
]);
95+
final String capturedStdout = results[0] as String;
96+
final int exitCode = results[2] as int;
97+
98+
if (exitCode != 0) {
99+
final String desc = description ?? '$executable ${arguments.join(' ')}';
100+
throw Exception('$desc failed (exit $exitCode)');
101+
}
102+
return capturedStdout;
66103
}

0 commit comments

Comments
 (0)