dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.71k stars 1.07k forks source link

CLI Usage of stdout v. stderr #7289

Open TheRealPiotrP opened 7 years ago

TheRealPiotrP commented 7 years ago

We need to identify how CLI takes advantage of the stdout and stderr streams to set good expectations and help folks consume the CLI effectively, particularly in pipelining scenarios. I'd love to first get the community's thoughts on what stdout and stderr mean to them. Once we come up with guidance we want to follow we can use the issue to track cleaning up the CLI and its scripts.

/cc a bunch of folks who will have opinions @nguerrera @juergenpf @blackdwarf @glennc @jaredpar @rainersigwald @MattGertz @davidfowl @eerhardt

blackdwarf commented 7 years ago

I would say, from my perspective, that we should use these two streams as they've been designed, that is:

Another thing we should consider doing, to be good *nix citizens, is to identify dotnet in the error messages, that is, prefix each error message with dotnet:.

TheRealPiotrP commented 7 years ago

@blackdwarf I've failed to find a good explanation of STDOUT vs. STDERR

@nguerrera brought up an interesting point that STDOUT may benefit from being pipelined to other apps/scripts. STDERR, in this scenario, would get things like diagnostic messages.

I think this topic merits some discussion and I'd love to get some community feedback before we write down what we intend.

blackdwarf commented 7 years ago

@piotrpMSFT http://www.jstorimer.com/blogs/workingwithcode/7766119-when-to-use-stderr-instead-of-stdout is a pretty nice explanation.

TL;DR: STDERR is for diagnostic messages, while STDOUT is for normal app output. Since they are separated, they can be redirected independently, which means that if I redirect STDOUT into a file, STDERR will still show error messages on the console. Actually, this use case was the primary reason the stream was introduced. 🙂

Of course, I'm not saying we should do as I specified, I was just sharing my opinion on the topic.

vcsjones commented 7 years ago

I would say, from my perspective, that we should use these two streams as they've been designed, that is:

  • Operational output goes to STDOUT
  • Errors and similar go to STDERR

Yes I agree. Certain build tools and scripts treat output from each distinctly. For example, if we receive output on stderr, the build server may assume that the build has failed (in addition to checking the exit code).

Consider if we had a very noisy build on STDOUT, but we still wanted to receive STDERR to a log file. Perhaps we would do something like dotnet myapp.dll > /dev/null 2> error.log. This would throw away the "noise" but log all of the STDERR to the error.log file. Another option is dotnet myapp.dll > /dev/null which doesn't redirect STDERR, so it prints to the console normally, but STDOUT is thrown out.

jaredpar commented 7 years ago

When making the decision for CLI I think we should take a look at our existing tooling and see if it sets up a pattern here. Looking at the tools involved in the CLI toolchain I know for instance the compilers never use StdErr. Even error messages go to stdout.

I believe MSBuild is the same but wouldn't swear to it.

rainersigwald commented 7 years ago

MSBuild does put everything on stdout. But I don't think that's the right design, and I'd rather go with the industry standard (primary output to stdout, warnings and errors to stderr) for any new tool.

jaredpar commented 7 years ago

@rainersigwald Every MS tool I can find for dotnet uses stdout for all output. Why have this tool be an outlier in our dotnet pipeline?

bricelam commented 7 years ago

Instead of asking what to log where using broad categories like "warnings" and "primary output" (no idea what that means), shouldn't we start at the consuming end: What text would I actually want to pipe to another command, or what text would get in the way of that?

bricelam commented 7 years ago

My opinion: Differentiating between stdout and stderr only makes sense when all of stdout is expected to be in a well-defined, consumable format, but you also need to report errors that don't match that format. (Think of tools like diff or ls.) Otherwise, I don't really see the point of using stderr at all. As stated, it just creates a big hassle with things that treat anything on stderr as an error (even though it may just be a warning or diagnostic information).

WernerMairl commented 7 years ago

i like the discussions about this - i see the same question in a lot of other (console) projects and i hope that we come to a design decision that can be documented in one of the design repositories!

One aspect that comes in mind: i see a lot of console applications (including msbuild) that are using colorized output (red for errors, yellow for warnings etc.)

In my understanding (not 100% verified) , ConsoleColor can only be used with Console.WriteLine and Console.WriteLine writes always and only to stdout.

Sample: ConsoleLogger from ASP.Net LoggerFactory (Microsoft.Extensions) is using colorized Output for Trace and Debug Messages. Trace and Debug should (Unix-Convention) go to stderr, currently i see no way to write to stderr in a colorized way => feature loss.....

I can imagine, that one reason for non using stderr in the past was the easy usage of colorized (red) outpout on stderr.....

my personal long term vision for this:

ghost commented 6 years ago

If compilation fails, at least the error code should indicate that, so downstream scripts can make decisions?

# in my script
msbuild non-existing.sln

if (!$?) {
  echo "this will never hit"
}

Comparing it with a popular tool:

# powershell
git blah

if (!$?) {
  echo  "blah is not a command dude!"
}

git status

if (!$?) {
  echo  "if you are reading this, that means you ran this outside of git repository"
}
rainersigwald commented 6 years ago

@kasper3 MSBuild sets errorlevel, and I don't repro that:

s:\msbuild>msbuild non-existing.sln
Microsoft (R) Build Engine version 15.7.66.2115 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

MSBUILD : error MSB1009: Project file does not exist.
Switch: non-existing.sln

s:\msbuild>echo %ERRORLEVEL%
1

s:\msbuild>powershell
Windows PowerShell
Copyright (C) Microsoft Corporation. All rights reserved.

Loading personal and system profiles took 2137ms.
S:\msbuild [core-node-reuse +0 ~2 -0 !]> msbuild non-existing.sln
Microsoft (R) Build Engine version 15.7.66.2115 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

MSBUILD : error MSB1009: Project file does not exist.
Switch: non-existing.sln
S:\msbuild [core-node-reuse +0 ~2 -0 !]> echo $?
False
jasonkarns commented 4 years ago

Sharing my experience (which brought me to the repo to search for precisely this issue)

My attempt to iterate over migrations so that I could progressively apply-then-rollback failed because the build output is first sent to stdout (Build started... Build succeeded.)

for m in $(dotnet ef migrations list 2>/dev/null); do

Second scenario: naively intuitive attempt to generate the migration script fails for the same reason (diagnostic build information also goes to stdout).

dotnet ef migrations script 2>/dev/null > migrate.sql

Both of these scenarios were the first thing I reached to as a long-time *nix/CLI user. That is what is conventional in the ecosystem and is what most (nix/cli) users would reach to intuitively. That these norms are broken makes the tool feel broken or at least unpolished and out of place. It gives one a feeling that the cli is a second-class experience (the first-class experience assumed as "use Visual-Studio-on-Windows").

bricelam commented 4 years ago

@jasonkarns Can you submit a new issue on dotnet/efcore? I've had similar thoughts recently on making these commands more scriptable. Currently, the best way to consume these commands is to use the --prefix-output and/or --json options combined with tools like grep or awk. Definitely not ideal.

jasonkarns commented 4 years ago

@bricelam sure can! I had thought about doing that, but assumed that since most of the undesirable output was coming from msbuild, I assumed this would need to be handled by the cli. I'll get an issue written up soon, though, thanks!

Adding another "broken" scenario so I don't lose it:

dotnet ef database --project MyProj --context ContextA update --no-build

This command executes successfully by returning an exit code of zero; but the command fails to do anything. So the exit status is misleading in a catastrophic way: shell scripts using this command continue as if the command were successful!

bricelam commented 4 years ago

IIRC, we already intercept all that output using ProcessStartInfo.RedirectStandardOutput. We can very easily write it to stderr instead.

akdor1154 commented 3 years ago

Got hit by this with a simple dotnet run - I'm writing a CLI script that outputs some CSV. dotnet run -- input.csv > output.csv Uh oh, what's that up the top of output.csv? output.csv

/home/jarrad/src/personal/lve_upload/Program.fs(233,13): warning FS0058: Possible incorrect indentation: this token is offside of context started at position (232:30). Try indenting this token further or using standard formatting conventions. [/home/jarrad/src/personal/lve_upload/lve_upload.fsproj]
/home/jarrad/src/personal/lve_upload/Program.fs(233,13): warning FS0058: Possible incorrect indentation: this token is offside of context started at position (232:30). Try indenting this token further or using standard formatting conventions. [/home/jarrad/src/personal/lve_upload/lve_upload.fsproj]
/home/jarrad/src/personal/lve_upload/Program.fs(234,24): warning FS0058: Possible incorrect indentation: this token is offside of context started at position (232:30). Try indenting this token further or using standard formatting conventions. [/home/jarrad/src/personal/lve_upload/lve_upload.fsproj]
name,email
akdor,akdor@akdor.akdor

So in my case it would at least make sense for build output to go to stderr in the case of dotnet run!

yecril71pl commented 2 years ago

Operational output goes to STDOUT

I do not know what you mean by operational output. Standard output is for the result of the command. That result should be specified and contain no gibberish.

In particular, the expected result for MSBuild is empty.

esmadau commented 2 years ago

Just wanted to drop a line to say that this is definitely having an effect on my team and trying to do anything with the CLI during CI/CD pipelines. This issue was first opened with that use case on mind. We need to be able to automate this. My current option is parsing for exact output used to throw errors myself. Even for something like 'dotnet foo', it'd be great to have stderr output.

baronfel commented 2 years ago

@lagomesma are there specific commands that we should focus on the semantics of? I can get on board an interpretation of "dotnet run should map all build related output to stderr" because to do otherwise removes the ability of the app being run to decide on the semantics of stdout. I'm wondering if there are other commands that should do the same, or if run is the source of most folks' pain.

esmadau commented 2 years ago

@baronfel , I'd like to see stderr output on errors on build as well. It seems like this is affecting commands built to run with dotnet as well like 'swagger'. So checking for swagger breaking API changes, I'd expect a failure when those changes are detected and I see there's an error. I submitted an issue for that here with more details.
A similar issue was logged by another issue for dotnet format --verify-no-changes here.

Ideally, we'd have and work towards consistency across all commands, but I understand if you'd like to start with one and expand to others.

baronfel commented 2 years ago

Thanks for the feedback @lagomesma!

I definitely agree that there's a place here for guidance around the proper use of stdout/stderr for tools that ship in or around the SDK (like run, format, etc), as well as recommendations for CLI application authors generally (which I'd include swagger in).

I think one major source of trouble for Powershell users especially is that Powershell historically hasn't had a great interaction between the status codes of 'native applications' and Powershell's own error-handling utilities like try/catch. While diggin into your linked issue I discovered that Powershell 7.3 (currently in preview) will help unify this behavior:

>$PSNativeCommandUseErrorActionPreference=$true
>$ErrorActionPreference = "Stop"
> try { dotnet swagger } catch { Write-Host "BOOM" }
You must install or update .NET to run this application.

App: C:\Users\chethusk\.nuget\packages\swashbuckle.aspnetcore.cli\6.0.7\tools\net5.0\any\dotnet-swagger.dll
Architecture: x64
Framework: 'Microsoft.AspNetCore.App', version '5.0.0' (x64)
.NET location: C:\Program Files\dotnet\

The following frameworks were found:
  2.1.30 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  6.0.5 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  6.0.7 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  7.0.0-rc.1.22402.14 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]

Learn about framework resolution:
https://aka.ms/dotnet/app-launch-failed

To install missing framework, download:
https://aka.ms/dotnet-core-applaunch?framework=Microsoft.AspNetCore.App&framework_version=5.0.0&arch=x64&rid=win10-x64
BOOM

With Powershell 7.3 installed and configured the pain point you identified is solved.

All of that doesn't change that we could be more consistent, but the dotnet host itself is writing to stderr currently. I was able to redirect the error stream to a file and the output of my terminal was much more manageable:

>  try { dotnet swagger 2>error.txt } catch { Write-Host "BOOM" }
BOOM
> cat error.txt
You must install or update .NET to run this application.

App: C:\Users\chethusk\.nuget\packages\swashbuckle.aspnetcore.cli\6.0.7\tools\net5.0\any\dotnet-swagger.dll
Architecture: x64
Framework: 'Microsoft.AspNetCore.App', version '5.0.0' (x64)
.NET location: C:\Program Files\dotnet\

The following frameworks were found:
  2.1.30 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  6.0.5 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  6.0.7 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  7.0.0-rc.1.22402.14 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]

Learn about framework resolution:
https://aka.ms/dotnet/app-launch-failed

To install missing framework, download:
https://aka.ms/dotnet-core-applaunch?framework=Microsoft.AspNetCore.App&framework_version=5.0.0&arch=x64&rid=win10-x64

This is in line with what I'd expect the behavior to be.

esmadau commented 2 years ago

Nice find! So dotnet does provide some type of error code on that output as a "native app" and it's just that Powershell's try catch is not respecting it?

baronfel commented 2 years ago

From what I can tell, yes! And this is something that I'd expect to be the case with all commands that aren't Powershell Cmdlets specifically

gortok commented 1 year ago

This is an issue that should have been addressed in 2016 at the moment it was decided .NET Core was going to be cross platform. This is a major problem for those of us that use the .NET CLI for build runners that are POSIX compliant.

Heck, it’s even a problem against Azure Devops. Here’s a common scenario:

Use dotnet pack and publish to publish a nuget package. If that package successfully pushes (does not have a duplicate version) then tag the source tree. You can’t do that easily in Azure devops against “failOnStdErr” because the dotnet cli does not respect stderr.

If there is an error we want that to go to stdErr. Period. That’s how automated runners are set up to run triggers or actions, and it sounds like the alternative is for us to constantly write scripts that parse STDOUT for what we need. That’s ridiculous.