Skip to content

Commit

Permalink
Modify line reader to always consume stderr and mark stderr in callba…
Browse files Browse the repository at this point in the history
…ck (#560)

This PR modifies the behavior of lineReader which would previously only
tried on reading from stderr when requested.

Since typically stderr is used to pass on important information and
given almost all uses in out codebase would set stderr as `true`, this
method now connects to stderr by default.

For the use-cases where we want to be able to tell whether line comes
from stderr or stdout (like with the sim-server stdout that is used as
communication and control channel), the line callback gets an additional
argument that indicates whether line comes from stderr.

## Test plan

Run expo-router, try clean rebuild on Android and iOS
  • Loading branch information
kmagiera authored Sep 19, 2024
1 parent 1e44788 commit 8f63403
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
2 changes: 1 addition & 1 deletion packages/vscode-extension/src/builders/buildAndroid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export async function buildAndroid(
);
const buildAndroidProgressProcessor = new BuildAndroidProgressProcessor(progressListener);
outputChannel.clear();
lineReader(buildProcess, true).onLineRead((line) => {
lineReader(buildProcess).onLineRead((line) => {
outputChannel.appendLine(line);
buildAndroidProgressProcessor.processLine(line);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/vscode-extension/src/builders/buildIOS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export async function buildIos(

const buildIOSProgressProcessor = new BuildIOSProgressProcessor(progressListener);
outputChannel.clear();
lineReader(process, true).onLineRead((line) => {
lineReader(process).onLineRead((line) => {
outputChannel.appendLine(line);
buildIOSProgressProcessor.processLine(line);
// Xcode can sometimes escape `=` with a backslash or put the value in quotes
Expand Down
11 changes: 8 additions & 3 deletions packages/vscode-extension/src/devices/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,21 @@ export class Preview implements Disposable {

const streamURLRegex = /(http:\/\/[^ ]*stream\.mjpeg)/;

lineReader(subprocess).onLineRead((line) => {
lineReader(subprocess).onLineRead((line, stderr) => {
if (stderr) {
Logger.info("sim-server:", line);
return;
}

const match = line.match(streamURLRegex);

if (match) {
Logger.debug(`Preview server ready ${match[1]}`);
Logger.info(`Stream ready ${match[1]}`);

this.streamURL = match[1];
resolve(this.streamURL);
}
Logger.debug("Preview server:", line);
Logger.info("sim-server:", line);
});
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vscode-extension/src/project/metro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export class Metro implements Disposable {
reject(new Error("Metro exited but did not start server successfully."));
});

lineReader(bundlerProcess, true).onLineRead((line) => {
lineReader(bundlerProcess).onLineRead((line) => {
try {
const event = JSON.parse(line) as MetroEvent;
if (event.type === "bundle_transform_progressed") {
Expand Down
20 changes: 10 additions & 10 deletions packages/vscode-extension/src/utilities/subprocess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Logger } from "../Logger";
import execa, { ExecaChildProcess } from "execa";
import readline from "readline";
import { Platform } from "./platform";
import { inc } from "semver";

export type ChildProcess = ExecaChildProcess<string>;

Expand Down Expand Up @@ -62,26 +63,25 @@ function overrideEnv<T extends execa.Options>(options?: T): T | undefined {
* When using this methid, the subprocess should be started with buffer: false option
* as there's no need for allocating memory for the output that's going to be very long.
*/
export function lineReader(childProcess: ExecaChildProcess<string>, includeStderr = false) {
const input = childProcess.stdout;
if (!input) {
throw new Error("Child process has no stdout");
export function lineReader(childProcess: ExecaChildProcess<string>) {
if (!childProcess.stdout) {
throw new Error("Child process doesn't have stdout");
}
const stdoutReader = readline.createInterface({
input,
input: childProcess.stdout,
terminal: false,
});
let stderrReader = null;
if (includeStderr && childProcess.stderr) {
let stderrReader: readline.Interface | null = null;
if (childProcess.stderr) {
stderrReader = readline.createInterface({
input: childProcess.stderr!,
input: childProcess.stderr,
terminal: false,
});
}
return {
onLineRead: (callback: (line: string) => void) => {
onLineRead: (callback: (line: string, stderr?: boolean) => void) => {
stdoutReader.on("line", callback);
stderrReader?.on("line", callback);
stderrReader?.on("line", (line) => callback(line, true));
},
};
}
Expand Down

0 comments on commit 8f63403

Please sign in to comment.