crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.48k stars 1.62k forks source link

Anticipating the lack of standard streams in the standard library #13330

Open HertzDevil opened 1 year ago

HertzDevil commented 1 year ago

The standard library assumes that the standard streams (STDIN, STDOUT, STDERR) are always usable, but this is often not the case; the streams may be redirected to null (e.g. Android apps only have access to various log streams), or may be invalid (e.g. graphical mode Windows applications). The standard library therefore should avoid hardcoded uses of those streams, or at least minimize them and make them easily redefinable.

To my knowledge, STDIN is never directly used by the standard library. STDOUT is used by CLI components like Benchmark and Spec to print normal information. STDERR is used for warnings, like Unsupported BIO ctrl call, and error information, like stack traces for unhandled exceptions. This means we will need at least 3 methods for these uses:

module Crystal
  def self.info(str)
    STDOUT.puts str
  end

  def self.warn(str)
    STDERR.puts str
  end

  def self.fatal(str, status = 1) : NoReturn
    STDERR.puts str
    STDERR.flush
    LibC.exit(status)
  end
end

A Windows GUI application might then do:

module Crystal
  def self.info(str)
  end

  def self.warn(str)
  end

  def self.fatal(str, status = 1) : NoReturn
    LibC.MessageBoxW(nil, str.to_utf16, "Unhandled exception".to_utf16, 0x10)
    LibC.exit(status)
  end
end

Lib.MessageBoxW shows a modal dialog, and actually doing the above means that the current program isn't terminated until the user dismisses the dialog. This is probably the more desirable behavior for GUI programs in general as well, not just Windows ones. On the other hand, .info and .warn must not interrupt the application in any way.

The main disadvantage of this approach is that Exception#inspect_with_backtrace has no IO to print to, so it must go through an interpolated string or a temporary String::Builder. The code for unhandled exceptions in an interrupt handler:

https://github.com/crystal-lang/crystal/blob/14bfa992ebd9cfa8946a151065dff7eebf829333/src/crystal/system/win32/process.cr#L142-L145

would become:

Crystal.fatal(String.build do |io|
  ex.inspect_with_backtrace(io)
  io.puts "FATAL: uncaught exception while processing interrupt handler, exiting"
end)

(Also LibC.exit performs resource cleanup in the C runtime and LibC._exit doesn't. We should settle on one of these.)

Those methods make no assumptions about whether the Crystal or C runtime is in a consistent state. As they may reference the standard stream constants, some lower-level error messages must continue to use Crystal::System.print_error. That method might need to be publicly redefinable as well, since the method assumes the availability of file descriptor 2.

straight-shoota commented 1 year ago

Sounds good. I don't think not being able to write directly to an IO is much of a disadvantage. Performance is no issue in a fatal error handler. And I presume allocations need to happen anyway in order to produce a means for displaying the message (i.e. MessageBoxW).

HertzDevil commented 1 year ago

I am thinking if there is a functional difference between ::puts and Crystal.info, and whether we really need the latter. For example, if I really want to run a benchmark inside an Android app, I could do this on top of android-ndk.cr:

require "android_ndk/log"

module Crystal
  def self.info(str)
    AndroidNDK::Log.write(:info, "crystal", str.to_s)
  end
end

Now the log stream is very different from a console stream; it most certainly doesn't support virtual sequences, and every single write call emits a new log entry with its own datetime and tag etc. Benchmark and Spec (--verbose) both rely on the ability to erase previously written text, so they simply don't make a lot sense when run inside an Android app. A similar conclusion can be drawn if Crystal.info "redirects" text to a stdout.txt file, which I have seen Windows GUI applications do. Do we accept this as the reality?

Second, it becomes tempting for users to either use Crystal.info directly instead of ::puts, or redefine the latter in terms of the former, blurring the difference between the two methods.

Then .fatal becomes the only necessity out of the 3 methods here.