Skip to content

Compilation error is not reported on stderr but stdout only #615

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

Closed
bsr203 opened this issue Sep 5, 2014 · 11 comments
Closed

Compilation error is not reported on stderr but stdout only #615

bsr203 opened this issue Sep 5, 2014 · 11 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@bsr203
Copy link

bsr203 commented Sep 5, 2014

During error, the message is set on stdout but not (also) on stderr. This is a change in behavior causes failure to show error message correctly in tools like grunt-ts. Is this intended.

More details on this bug
TypeStrong/grunt-ts#140

@mhegazy
Copy link
Contributor

mhegazy commented Sep 5, 2014

This is actually by design. writing errors to standard error was a wrong choice in the first place, as the errors are not errors from the compiler (e.g. crashes) but rather user errors, and the user would expect them as part of the normal output of the compiler.

The new behavior is consistent with other compilers as well.

@bsr203
Copy link
Author

bsr203 commented Sep 6, 2014

thanks for the quick response. From the grunt.util.spawn code in the referenced issue, in case of error it expect the message to be filled in stderr. I am not sure what is the convention in the node.js world. Probably @basarat may be better able to comment.

@basarat
Copy link
Contributor

basarat commented Sep 6, 2014

This is actually by design. writing errors to standard error was a wrong choice in the first place

👍 got it.

@bsr203 bsr203 closed this as completed Sep 6, 2014
@RyanCavanaugh RyanCavanaugh added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Sep 8, 2014
@bdkjones
Copy link

@mhegazy I'm not sure what other compilers you're referencing, but all of the following write compiling errors to StdErr:

  1. Less
  2. Sass
  3. Stylus
  4. CoffeeScript
  5. HAML
  6. Jade
  7. Compass
  8. Slim
  9. Autoprefixer
  10. Uglify.js

TypeScript is definitely the odd one out. The convention in the node.js world is to use StdErr.

I understand your argument that "syntax error messages are actually just output". But that's very much an academic computer-science-class argument. In the real world, the general rule is to use StdErr whenever the output is not something you would want to pass to the next process that's going to use the StdOut pipe. If the next process is expecting the compiled JS to come through StdOut and you throw a textual error message down the pipe, that's not correct, as explained here: http://www.jstorimer.com/blogs/workingwithcode/7766119-when-to-use-stderr-instead-of-stdout

It would be far better to use StdErr for that error message, even though the compiler technically ran correctly and just found syntax issues in the user's file. This is the natural expectation and the convention followed by everyone except TypeScript (although TS used to do this correctly).

Just my two cents. I'd like to see this decision reversed.

@basarat
Copy link
Contributor

basarat commented Mar 30, 2015

I'm not sure what other compilers you're referencing

C# / F# / VB.Net

Neither defending nor denouncing. Just mentioning 👍

@bdkjones
Copy link

It's also worth pointing out that the CLI returns a non-zero value when a syntax issue is discovered in a file. If your argument is that a syntax error should go to StdOut because it's just "normal output", then the CLI should return 0 in those cases because it "ran normally".

(This is a terrible idea---if you actually implemented that, there'd be no way for implementors like me to determine if TS compiled successfully. I simply point this out to highlight the inconsistency of using StdOut when the return code is non-zero.)

It's a pretty universal pattern:

  1. Run [thing].
  2. Did [thing] return 0?
  3. Nope.
  4. Ok, see what's on StdErr.

TS breaks this. At the very least, I'd say that syntax errors should produce output on both pipes.

@basarat
Copy link
Contributor

basarat commented Mar 30, 2015

@bdkjones very valid argument. I'll use stderr if I ever design a compiler ❤️

@bdkjones
Copy link

@basarat Yea, there are two different perspectives: the TS user and the TS-compiler-developer.

For the user, a "normal" run of the compiler results in a JavaScript file. Anything else is an error and therefore NOT a normal run of the compiler.

For the folks that develop the TypeScript compiler, a "normal" run of the compiler is anything that doesn't crash. Error messages are simply one of several "normal endings" for a run of the compiler.

@mhegazy has chosen to adopt the second perspective. I believe that's a poor choice because it doesn't match what the user expects. To the folks that use the TS compiler, an error message is NOT "normal output". Normal output is a JavaScript file. Anything else is a problem.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 30, 2015

@bdkjones What is the scenario you are trying to achieve that is currently broken by not writing errors to stderr?

@bdkjones
Copy link

Well, I build an app called CodeKit that, among other things, compiles TS files. My users suddenly reported that my app no longer displayed error messages from the TS compiler. When I investigated, I found that you guys switched from stderr to stdout, without mentioning it. That's really a breaking change and it wasn't handled with the appropriate deprecation procedure (at least, none that I found). I can deal with your choice to use stdout, I just think it's A) not correct and B) was done without any regard to the impact it would have on people who have built workflows around your compiler.

Sent from my iPhone

On Mar 30, 2015, at 09:54, Mohamed Hegazy [email protected] wrote:

@bdkjones What is the scenario you are trying to achieve that is currently broken by not writing errors to stderr?


Reply to this email directly or view it on GitHub.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 30, 2015

I am sorry about the lack of notification for this breaking change. That has been done as part of our complete rewrite between 1.0 and 1.1. Some of these issues have slipped through without following the breaking change notification process. If any consolation, we have been tracking breaking changes more diligently since then. As for emitting errors to stderr, I think we would want to avoid another breaking change at this point, and stick with stdout.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

5 participants