Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CommandLineRunner/csi should emit errors & diagnostics to STDERR, not STDOUT #9826

Closed
atifaziz opened this issue Mar 17, 2016 · 2 comments
Closed
Assignees

Comments

@atifaziz
Copy link
Contributor

Version Used: 1.1.0.51109

Steps to Reproduce:

See #9791

Open a Command Prompt on 64-bit Windows and enter and execute the following line on the prompt:

echo throw new Exception("BOOM!");> test.csx && "%ProgramFiles(x86)%\MSBuild\14.0\bin\csi.exe" test.csx 2>log.txt || echo NOK

Expected Behavior:

That only NOK should be displayed on STDOUT and the exception text is written to log.txt since STDERR has been redirected to that file.

Actual Behavior:

At the end of execution log.txt is empty, and both NOK and the exception text are written to STDOUT:

System.AggregateException: One or more errors occurred. ---> System.Exception: BOOM!
   at Submission#0.<<Initialize>>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Scripting.ScriptExecutionState.<RunSubmissionsAsync>d__9`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Scripting.Script`1.<RunSubmissionsAsync>d__19.MoveNext()
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at Microsoft.CodeAnalysis.Scripting.Hosting.CommandLineRunner.RunScript(ScriptOptions options, String code, ErrorLogger errorLogger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Scripting.Hosting.CommandLineRunner.RunInteractiveCore(ErrorLogger errorLogger)
   at Microsoft.CodeAnalysis.Scripting.Hosting.CommandLineRunner.RunInteractive()
   at Microsoft.CodeAnalysis.CSharp.Scripting.Hosting.Csi.Main(String[] args)
---> (Inner Exception #0) System.Exception: BOOM!
   at Submission#0.<<Initialize>>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Scripting.ScriptExecutionState.<RunSubmissionsAsync>d__9`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Scripting.Script`1.<RunSubmissionsAsync>d__19.MoveNext()<---

NOK

For more background, see #9791 from which this issue was created on request of @tmat.

The fix in PR #9791 only addresses part of the issue.

@atifaziz
Copy link
Contributor Author

Then of course tests need to be updated.

@tmat TestConsoleIO adds color markers in the output that are then tested in CommandLineRunnerTests as part of the output (e.g. see line 183). I was thinking of adding «StdErr» and «StdOut» markers in similar vein. Would that be a reasonable approach?

@tmat
Copy link
Member

tmat commented Mar 17, 2016

Do we need to change the tests at all? We can leave the existing tests using the same stream for both Error and Out. That way we are testing the common user experience on the console.

Then we can have just one or two tests that separate the streams and verify content of each stream separately.

@atifaziz atifaziz changed the title csi.exe should emit exception to STDERR, not STDOUT csi/CommandLineRunner should emit errors & diagnostics to STDERR, not STDOUT Mar 18, 2016
@atifaziz atifaziz changed the title csi/CommandLineRunner should emit errors & diagnostics to STDERR, not STDOUT CommandLineRunner/csi should emit errors & diagnostics to STDERR, not STDOUT Mar 18, 2016
@ManishJayaswal ManishJayaswal added this to the 2.0 (RC) milestone Apr 26, 2016
@tmat tmat closed this as completed in 92999b7 May 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants