Node launcher doesn't work when user doesn't specify any node options.

[uri-canva]

🐞 bug report

Affected Rule

nodejs_binary

The issue is caused by the rule:

Is this a regression?

Yes, it was working in 1.0.1, I believe it was introduced in b10d230.

Yes, the previous version in which this bug was not present was: ....

Description

A clear and concise description of the problem...

If the user doesn't specify any node options, the launcher will execute "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" "${ARGS[@]:-}", note the quotes around USER_NODE_OPTIONS: if the array is empty it will expand to an empty string. That will run the node interpreter with no script, which does nothing and exits 0.

🔬 Minimal Reproduction

Create a nodejs_binary without any node options that always fails, run it, observe it succeeds.

🔥 Exception or Error

🌍 Your Environment

Operating System:

  
Darwin MacBook-Pro-108.local 19.3.0 Darwin Kernel Version 19.3.0: Thu Jan  9 20:58:23 PST 2020; root:xnu-6153.81.5~1/RELEASE_X86_64 x86_64
  

Output of bazel version:

  
Build label: 2.0.0- (@non-git)
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Fri Jan 10 19:03:57 2020 (1578683037)
Build timestamp: 1578683037
Build timestamp as int: 1578683037
  

Rules_nodejs version:

(Please check that you have matching versions between WORKSPACE file and @bazel/* npm packages.)

  
1.4.1
  

Anything else relevant?

A similar issue with the arguments was fixed in #1650.

[uri-canva]

cc @gregmagolan, @purkhusid

[ClareCat]

We just ran into this too. It looks like it was fixed in #2017 and added to 2.0.0-rc.2

I added the following patch to 1.7 which seems to have fixed the issue for us:

index 61eec6d1..9efd6355 100644
--- a/internal/node/launcher.sh
+++ b/internal/node/launcher.sh
@@ -260,12 +260,12 @@ if [ "${EXPECTED_EXIT_CODE}" -eq "0" ]; then
   # handled by the node process.
   # If we had merely forked a child process here, we'd be responsible
   # for forwarding those OS interactions.
-  exec "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"}
+  exec "${node}" ${LAUNCHER_NODE_OPTIONS[@]+"${LAUNCHER_NODE_OPTIONS[@]}"} ${USER_NODE_OPTIONS[@]+"${USER_NODE_OPTIONS[@]}"} "${MAIN}" ${ARGS[@]+"${ARGS[@]}"}
   # exec terminates execution of this shell script, nothing later will run.
 fi

 set +e
-"${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"}
+"${node}" ${LAUNCHER_NODE_OPTIONS[@]+"${LAUNCHER_NODE_OPTIONS[@]}"} ${USER_NODE_OPTIONS[@]+"${USER_NODE_OPTIONS[@]}"} "${MAIN}" ${ARGS[@]+"${ARGS[@]}"}
 RESULT="$?"
 set -e

[alexeagle]

Sounds like this is already fixed in upcoming release?

[github-actions]

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"