Sdk: Add the ability to capture standard output

[nex3]

The IOOverrides class makes it possible to override a number of dart:io interfaces, but it doesn't support stdout or stderr. This makes it impossible to capture all the output from a chunk of Dart code. This is useful for (among other things) writing persistent Bazel workers that wrap a Dart program's main() method and send its output as a protobuf to the build runner.

This is blocking Dart Sass's ability to support Bazel workers within Google, which is high-priority for us.

[matanlurey]

Just casually curious why this is blocking - there are a number of persistent bazel workers without this functionality being implemented

[nex3]

The existing Dart workers I'm familiar with either use Scissors' generic Bazel worker, which spawns a subprocess and so has total control over how its stdin and stderr are handled, or they interact with a Dart library that has APIs for controlling logging. We need to invoke the Sass worker in-process because the point of the worker is to keep a hot VM, but we need to invoke its main() method because we're passing in the same command-line flags that get passed to the normal executable.

[matanlurey]

Right. I would talk to @jakemac53 or @natebosch - they have implemented a bunch of these without any special SDK help.

[nex3]

I believe those all fall under the second category--they use the build package's API rather than invoking a main() method.

[natebosch]

Yeah we do everything with a Zone capturing print calls. In all of our usage if the other code used stdout directly it would cause problems, so we disallow it.

Why does Sass need to use stdout instead of print?

[jakemac53]

Fwiw the existing functionality here in package:build is inherently flaky due to this - the only thing we can override right now is the print method but if a Builder or any of its dependencies directly logs to stdout that will break the worker.

In practice this hasn't been much of an issue because we encourage people to go through our logging apis already, but it is a potential source of extremely difficult to debug issues. We would welcome a change here.

[nex3]

Why does Sass need to use stdout instead of print?

We use print(), but we also use stderr for logging warnings and errors. Capturing that is a bigger priority, but being able to capture stdout as well would be useful for ensuring that we don't accidentally end up writing output in a way that's invisible.

[jakemac53]

but being able to capture stdout as well would be useful for ensuring that we don't accidentally end up writing output in a way that's invisible.

More than just this - the blaze workers communicate on stdin/stdout and writing arbitrary output there will cause blaze to kill the worker and throw an error.

[jakemac53]

Fwiw though - I wouldn't consider this to be blocking for implementing sass workers.

Providing an abstraction for logging is useful anyways, and you shouldn't need to invoke main directly just to support command line arg parsing. You can instead expose a public method that takes command line args and parses them into an options object or something along those lines.

[nex3]

I don't want to support that as part of Sass's public API. It's not generally useful, especially since that should be able to work by importing the script file and invoking main().

The IOOverrides API was added specifically to avoid making users create manual abstractions for IO operations. Extending that to stdout and stderr (which most languages allow to be captured) doesn't seem like a big ask.

[davidmorgan]

+1 this would be a significant improvement--allowing us to package code as workers without having to mess with the internals.

[vmutafov]

Any updates on this?

[davidmorgan]

I'm not sure anyone on the VM team actually saw this ... @mraleph this is the issue I mentioned that could have saved your bazel workers from dying when you added debug output to stdout ... do you think there is a chance of someone taking a look, please? Thanks!

[mraleph]

I don't think there are free resources for this at the moment.

/cc @a-siva, maybe @bkonyi can take a look at this at some point.

[mit-mit]

cc @sortie

[a-siva]

cc @ZichangG maybe another potential enhancement to be added to the list for the new cross platform packages team