feat: add support for capturing and overriding the exit code within run_node #1990
Conversation
| @@ -10,8 +10,10 @@ _ATTRS = { | |||
| "args": attr.string_list(mandatory = True), | |||
| "configuration_env_vars": attr.string_list(default = []), | |||
| "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]), | |||
| "exit_code_out": attr.output(), | |||
There was a problem hiding this comment.
exit_code on it's own seemed not descriptive enough, but can change these attr names to something better
internal/node/launcher.sh
Outdated
| # Captures the exit code of the node process to the file specified | ||
| --bazel_capture_exit_code=*) EXIT_CODE_CAPTURE="${ARG#--bazel_capture_exit_code=}" ;; | ||
| # Overrides the exit code of the node process | ||
| --bazel_override_exit_code=*) OVERRIDE_EXIT_CODE="${ARG#--bazel_override_exit_code=}" ;; |
There was a problem hiding this comment.
is there a case where you'd want to capture the exit code, and not have it exit zero? If we can't think of one, I think I'd rather keep the feature smaller just so there's less surface area for us to support.
There was a problem hiding this comment.
It seemed a natural additional option, but don't have a use case for it
There was a problem hiding this comment.
okay do you mind simplifying then, and just have the one option to capture, and always exit zero when the option is used?
Also we should document here that you still have to produce all the expected outputs from the program even when expecting a non-zero exit code, as we discovered in your PR that consumes this feature. That restriction does make it a lot less useful... but I'm not sure what else we can do in this model where the program and the error translator are separate actions.
There was a problem hiding this comment.
Will do 👍 Will add a note to the npm_package_bin docs too, I couldn't see where the run_node docs are surfaced, but I'll add something in there too I think.
mattem
force-pushed
the
feat/exit-code-capture
branch
from
d4f39f6
to
3a30db6
Compare
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the new behavior?
Adds
exit_code_outandoverride_exit_codetonpm_package_bin(and handling inrun_node) allowing the exit code of the binary to be captured to a file, and the subsequent exit code to then be overridden.