fix release proposal failing when stderr has output #5362
Conversation
Overall package sizeSelf size: 8.83 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.5.0 | 29.83 MB | 29.83 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.5.1 | 9.79 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.13.1 | 117.64 kB | 840.3 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5362 +/- ##
=======================================
Coverage 80.69% 80.69%
=======================================
Files 491 491
Lines 21860 21860
=======================================
Hits 17640 17640
Misses 4220 4220 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Datadog ReportBranch report: ✅ 0 Failed, 662 Passed, 0 Skipped, 11m 26.36s Total Time |
BenchmarksBenchmark execution time: 2025-03-04 22:34:24 Comparing candidate commit 396842d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 917 metrics, 16 unstable metrics. |
| if (result.status) { | ||
| throw new Error(stderr) |
There was a problem hiding this comment.
If result.status is not zero, and error will already have been thrown above on line 71, so this if-block can be removed.
| if (stderr) { | ||
| if (flags['no-abort-on-error']) { | ||
| log(`${RED}${stderr}${RESET}`) | ||
| } else { | ||
| fatal( | ||
| stderr, | ||
| 'Aborting due to error! Use --no-abort-on-error to ignore and continue.' | ||
| ) | ||
| } | ||
| } | ||
| } else if (treatStderrAsFailure && stderr) { | ||
| if (flags['no-abort-on-error']) { | ||
| log(`${RED}${stderr}${RESET}`) | ||
| } else { | ||
| fatal( | ||
| stderr, | ||
| 'Aborting due to error! Use --no-abort-on-error to ignore and continue.' | ||
| ) | ||
| } |
There was a problem hiding this comment.
Removing this will unfortunately get us back to the original problem, which prompted me to make this change in the first place: The branch-diff tool will exit with a status code zero, but output errors on STDERR if there's a permission issue.
While I have also updated the requirements check to check that the expected permissions are in place, it would still be nice to have some sort of guardrail in place for when branch-diff runs into issues.
However, instead of defaulting to throwing on output to STDERR, maybe we should change the default to ignoring it, and only for branch-diff commands we can enable it.
Alternatively, or in combination with this, we could also change the default to only output STDERR, but not abort on it.
What do you think about all this?
There was a problem hiding this comment.
Any way to force the command to throw when there is an error at the command level? A bit like when the output if redirected to /dev/null, but kind of the opposite of that.
There was a problem hiding this comment.
For now I will merge and we can address in another PR since this is blocking all releases.
What does this PR do?
Fix release proposal failing when stderr has output.
Motivation
It's common for
stderrto have output even without any error, which would end up failing the script. By instead relying on the status code, we can simplify the code and only abort on actual errors.