Add StdStream error reporting mechanism

SeanTAllen commented 2 years ago

In https://github.com/ponylang/ponyc/issues/4258 @pcrockett requested a means for getting reports of errors from StdStream.

The example given is taking a Pony hello world program and running it as such:

./hello-pony > /dev/full

Which should if we are being "fully correct" result in an exit code indicating an error as the hello world program wasn't able to "do its thing". See https://blog.sunfishcode.online/bugs-in-hello-world/ for more context.

There is currently no way if a programmer wanted to correctly report an error via the exit code as the programmer does not know if operations like Env.out.print succeeded or not.

There are 3 basic patterns for "reporting back asynchronous status" in Pony.

"notifier pattern"
promises
callback via explicit passing of a receiver to a call

Notifier concerns

The notifier pattern will not work here due to when/how Stdout and Stderr are set up unless, a notifier is added after a StdStream is created. The problem with notifier is that for any given StdStream, you can only notifier one actor. So a sending actor wouldn't necessarily know. This forces a programmer to have a per StdStream error receiver that might not be how they want to operate.

I don't think notifiers are a good option here.

Promises

A promise could be used. However what the usage would look like is unclear. Do we want to report success and failure? That seems unlikely, we probably only interested in failure, but perhaps not. If we only report failure, then to "clear the Promise supplied" we would need to reject the promise which is a weird API or we need a "sorry won't fulfill" option for a Promise which is also a weird API.

I don't think promises are a good option here.

Callback via passing explicit receiver

This would be something along the lines of:

use @pony_exitcode[None](code: I32)

interface StdStreamErrorReceiver
  be receive_stream_error(stream: StdStream)

actor Main
  new create(env: Env) =>
    env.out.print("Hello", this)

  be receive_stream_error(stream: StdStream) =>
    @pony_exitcode(1)

Where print would now be:

be print(data: ByteSeq, errors_to: StdStreamErrorReceiver)

There are a number of issues here:

the naming is awful, no one writing an RFC should use the names I slapped in above.
if printing fails, then works, we would to reset the exitcode. that suggests we need to understand "success" as well
sending messages for every success for every message printed is a massive overhead. perhaps it would be "only if last was error" or we have a second interface for "getting success"
we don't want this to be a breaking change so either there are new print, write etc methods that take an error receiver or the errors_to is (Interface | None) with a default of None.

There's a lot of questions about usability and design that would need to go into such an RFC.

A final thought: except where the notifier pattern is used, the standard library lacks the ability to get back any error or success status from actors that are part of the standard library. Consideration for "could this be expanded further into the standard library" should be addressed as part of any RFC.

jemc commented 2 years ago

If we go with the explicit receiver approach, and someone wanted to have a global error receiver in their application (without needing to worry about that concern everywhere that they print in the application), they'd want to create an abstraction layer that wraps the StdStream instance and implicitly inserts the explicit error receiver in the underlying call.

To make this work well, libraries that want to leverage output streams should use the abstract type (interface) OutStream rather than the concrete type StdStream, as the former would allow someone's wrapping abstraction to implement the OutStream interface and be supplied to a library that expects to receive an output stream to write to.

pcrockett commented 2 years ago

sending messages for every success for every message printed is a massive overhead

Yet I can see frequent success messages being an important feature, for example, if you're building an application that's concerned about writing data reliably. Knowing that "yes, this specific chunk of data did make it all the way to disk / your network peer / etc" seems like it would be important for a lot of people.

Many stream APIs in more traditional languages won't necessarily return simple success messages, but rather number of bytes in the buffer that were actually written. I could imagine a more "batched-oriented" system where one gets success messages for 100% of the data that they write, but those messages wouldn't necessarily be sent for every print call; rather, they'd be sent after a full N-byte chunk of data has been written, or as a result of an explicit flush.

Which brings up another point: flush is important to consider here as well.

Anyway, I could imagine something like this being pretty useful:

interface OutStreamProgressReceiver
  be receive_stream_progress(bytes_written: USize, stream: OutStream)

Note, bytes_written would be the number of bytes in a single chunk of data, not the total number of bytes written to the stream. It could use a better name.

SeanTAllen commented 1 year ago

We discussed this a bit today. Joe and I talked about rather than changing the existing stdstream that we opened up a way to allow people to "bring their own" implementation of "stdstream" functionality as one API might not work out well for everyone in advanced cases. For some full details (which would take this issue off track) you can listen to the December 6, 2022 pony sync recording. It's the first conversation that is part of the call.

ponylang / rfcs