Use case

Our current logging logic is quite convoluted:

if plutil has error {
  echoError(error) // use stderr, prints in verbose mode, or when exitError != 0
  if not verbose && exitCode == 0 { // condition where echoError is not print
    streamOutput(error) // so that we also print non-verbose mode & exitCode == 0
  }
}

Why do we write this code?

In xcode_backend.dart, echoError writes to stderr; then in mac.dart, build project calls _DefaultProcessUtils.run(), which prints runResult as trace, which is only printed in verbose mode:

In addition, when exitCode != 0, mac.dart's _parseIssueInStdout will print out stderr:

So with the above, stderr is only printed in verbose mode or when exitCode != 0. However, xcode_backend.dart wants to always print out that error, so it uses streamOutput for non-verbose & exitCode == 0.

Side Note: we check exitCode of current process, not result.exitCode of the subprocess (e.g. plutil). I think this check shouldn't be necessary, because if exitCode is not 0, then the current process is already terminated (xcode_backend never change exitCode without exiting). (TODO: file a separate issue)

Proposal

This is way too convoluted. For example, "verbosity" and "streaming" should have been orthogonal to each other. Why should "streaming" depend on verbosity? Why should "streaming" depend on exitCode?

Consider a more natural logic:

• If we want to print after process is complete, put it in stdout or stderr
• If we want to print immediately when it happens (aka streaming), use streamOutput
• Verbose mode prints everything. Non-verbose mode only prints errors

(Of course there's also "warning" which should be in stderr. I won't go into details here.)

For NSBonjour logic, we want to print immediately when it happens, so we would just call streamOutput (and not echoError):

if plutil has error {
  streamOutput(error)
}

Optional: split into 2 streams

We could even consider splitting into 2 streams: outStream and errStream, so we don't have to rely on "error:" prefix to distinguish them.

You may ask, "isn't it more complicated with 2 streams?". Well, with 1 stream, you still have 2 concepts (out and error), it's just that you re-use the same stream using "error:" prefix.

Having 2 streams is also more consistent with having separate stdout/stderr, so less cognitive overload

Note:
There's actually a logic error in our current 1 stream approach: (TODO: file a separate issue)

xcode_backend.dart uses "error:" prefix, but mac.dart checks whether the text contains "error:". So if a string has "error:" in the middle (rather than prefix), it would be recognized as an error.

An example of such bug would be plutil extract, which has "error" in the middle: Could not extract value, error: No value at that key path or invalid key path. We don't want to log it as error (since it's one of our normal states), so we didn't put an "error:" prefix. However, it's still printed out because it contains "error:" in the middle. See also: #173879

Of course, we can fix the logic error by just checking startsWith rather than contains in mac.dart. However, having 2 streams would have avoided this logic error in the first place.