aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.6k stars 3.9k forks source link

(core): (asset bundling message is printed to stderr) #23092

Open DanielAWood opened 1 year ago

DanielAWood commented 1 year ago

Describe the bug

Prior to asset bundling aws-cdk prints a message: "Bundling asset ...". This message is written to stderr despite not being an error itself. This leads to some build systems (Rush for instance) failing as they are confirming stderr in addition to the process return code.

Expected Behavior

stderr should be reserved for error messages

Current Behavior

"Bundling asset {path}" information message is printed to stderr

Reproduction Steps

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.51.1

Framework Version

No response

Node.js Version

v16.17.0

OS

Mac OSX 12.6

Language

Typescript

Language Version

No response

Other information

No response

mrgrain commented 1 year ago

I understand the predicament, but can you not redirect stderr in your rush commands? cdk synth 2>&1 or even cdk synth 2> /dev/null ?

DanielAWood commented 1 year ago

That doesn't appear to work in this case, and would risk losing valid errors otherwise I believe.

mrgrain commented 1 year ago

There's also --ci and the CI env variable that should do the same. LMK if these don't work and I can re-open the ticket.

github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

mrgrain commented 1 year ago

Okay, turns out --ci doesn't work at the moment.

We need to change this line to use the logger instead.

rittneje commented 1 year ago

Despite its name, stderr is not just for errors, but rather anything other than the "normal" output, such as diagnostic messages.

stdout should only be for the "normal" output. What exactly that is can vary from application to application, but generally it means the stable output of a program, and thus should not include log messages, regardless of their level.

A build system that assumes that output on stderr means there was an error or similar issue is making an incorrect assumption in violation of the standard. To tell if a command failed, it should exclusively look at the exit code.

mrgrain commented 1 year ago

Okay, turns out --ci doesn't work at the moment.

We need to change this line to use the logger instead.

So this won't be possible either because user's might also run cdk synth > template.yaml in a CI context that might have CI=1 enabled.


Thanks for the explanation @rittneje. I agree with this assessment.

Ideally we would still provide a way to silence all other output (read: not put it on a different stream, but just not print the Bundling asset ... message at all.)

Any implementation of this would need to find a different way. Usually --quiet is an option, but that is currently also suppressing the template output.

@DanielAWood Since your concern arose from running Template.fromStack(theStack); maybe there's a change you could make to ensure the internals use quiet here (and check that Bundling asset ... is actually suppressed by it).

PS: Changing this to a feature request.

DanielAWood commented 1 year ago

Thanks for the input