Skip to content
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

2.8.1: ExecJS.compile consistently exits 1 in one environment and 0 elsewhere. #105

Open
jrafanie opened this issue Aug 11, 2021 · 4 comments

Comments

@jrafanie
Copy link

jrafanie commented Aug 11, 2021

I have a few environments I'm testing execjs's compile in combination with uglifier and have a strange problem that happens consistently in one environment but not elsewhere. I have minimized the problem to this commit: a6abf13 released in 2.8.1 and can reliably recreate it with this script:

require 'bundler/inline'

gemfile do
  source 'http://rubygems.org'
  gem 'execjs',   "=2.8.1"
  gem 'uglifier', "=3.0.4"
end

require 'uglifier'
puts "Uglifier: #{Uglifier::VERSION}"
puts "ExecJS: #{ExecJS::VERSION}"

puts "Running test..."
ExecJS.compile(File.read(Uglifier::SourceMapPath)); nil
puts $?

In the bad environment, I'm seeing that this exits with 'ok' but with exit code 1:

Uglifier: 3.0.4
ExecJS: 2.8.1
Running test...
Traceback (most recent call last):

        7: from XXX/gems/execjs-2.8.1/lib/execjs/module.rb:27:in `compile'
        6: from XXX/gems/execjs-2.8.1/lib/execjs/runtime.rb:72:in `compile'
        5: from XXX/gems/execjs-2.8.1/lib/execjs/runtime.rb:72:in `new'
        4: from XXX/gems/execjs-2.8.1/lib/execjs/external_runtime.rb:14:in `initialize'
        3: from XXX/gems/execjs-2.8.1/lib/execjs/external_runtime.rb:39:in `exec'
        2: from XXX/gems/execjs-2.8.1/lib/execjs/external_runtime.rb:219:in `exec_runtime'
        1: from (execjs):1

ExecJS::RuntimeError (["ok"])
pid 134 exit 1

When I try to recreate this issue in other environments with 2.8.1, it exits correctly with exit code 0.

I determined it was this change: a6abf13 by testing this bad environment with execjs 2.8.0 and 2.7.0 without changing anything else and they both exit with exit code 0 as expected:

Uglifier: 3.0.4
ExecJS: 2.7.0
Running test...
pid 364 exit 0
Uglifier: 3.0.4
ExecJS: 2.8.0
Running test...
pid 249 exit 0

I'm using node 12.21.0 and it works on OSX and RHEL 8.4 and debian. The bad environment is also RHEL 8.4 based with a very similar setup as the working one.

It looks like we're explicitly calling process.exit() to avoid it hanging: c5fd11d. The docs say:

Rather than calling process.exit() directly, the code should set the process.exitCode and allow the process to exit naturally by avoiding scheduling any additional work for the event loop

The commit for 2.8.1 is such a minor change. I don't know if the process isn't gracefully terminating or some other low level error is happening but I cannot get more information to determine why this exits 1, which in turn makes execjs believe it failed. I've tried getting more output from node but the following options didn't give me anything extra:

--report-uncaught-exception
--report-on-fatalerror

Do you have any ideas on things I can try? Thanks!

@Fryguy
Copy link

Fryguy commented Aug 11, 2021

Note that we believed this was because the failing environment was FIPS enabled, but we've tried on other FIPS enabled envs and we can't duplicate it.

@jrafanie jrafanie changed the title 2.8.1: ExecJS.compile consistently exits 1 in one installation and 0 elsewhere. 2.8.1: ExecJS.compile consistently exits 1 in one environment and 0 elsewhere. Aug 11, 2021
@jkeam
Copy link

jkeam commented Aug 16, 2021

Looks like this might be the issue. Read on 🎉

This transpiled JS doesn't work (https://gist.githubusercontent.com/jrafanie/96475c6f34a55269e1add042bded92b0/raw/919de4331b9befa5ff3a1a22479b2a792d6b9a90/source_map_path_2.8.1.js)

And this one does (https://gist.githubusercontent.com/jkeam/141bb30cc0fdb5c42e21fd15c41b908d/raw/84ac502ad1bd3af7eb2bd42fbbc0b07bce78dda2/test_write.js)

The only difference between the two is that in the working version, we commented out this line:

delete this.setTimeout;

What it looks like based on our debugging session is that DynaTrace is trying to call the setTimeout method, but in the broken version that method does not exist and an error is thrown and the script returns a non-zero exit status code.

While not exactly the same issue, @radikaled found this issue #64 which has similar odd behavior of DynaTrace.

@Fryguy
Copy link

Fryguy commented Aug 16, 2021

As @jkeam said one thing we noticed was DynaTrace was installed on the openshift cluster - we are trying to evaluate whether that is actually what causes the issue, but it is looking likely suspect. We see the following stack trace which includes dynatrace, and separately we saw setTimeout removal involved.

sh-4.4$ node --trace-exit --trace-uncaught test281
["ok"](node:225) WARNING: Exited the environment with code 1
    at exit (internal/process/per_thread.js:173:13)
    at n (/opt/dynatrace/oneagent/agent/bin/1.219.162.20210726-100948/any/nodejs/nodejsagent.js:15740:49)
    at emit (events.js:314:20)
    at internal/process/execution.js:165:25

and

sh-4.4$ node --abort-on-uncaught-exception test281
["ok"]Uncaught ReferenceError: setTimeout is not defined

FROM
Illegal instruction (core dumped)

I see in the FAQ, that it talks about setTimeout, but what I can't understand from the README is why. That is not clear from the README, leading me to question why these methods are removed.

@jrafanie
Copy link
Author

It looks like one way to get around this issue is to use execjs 2.8.1 with uglifier 4.2.0+ since it defers the context call asset compilation so it doesn't happen on Uglifier.new which happens when specifying the assets.js_compressor in the rails application configuration. With uglifier 4.2.0 and execjs 2.8.1, we can pre-compile the assets in an environment without dynatrace, then at runtime we can avoid shelling out to nodejs, which was conflicting with dynatrace.

I'll update the issue if this doesn't work.

jrafanie added a commit to jrafanie/manageiq-ui-classic that referenced this issue Aug 17, 2021
ExecJS 2.8.1 requires uglifier 4.2.0 to defer uglifier asset compilation until asset
compilation time, see: rails/execjs#105

Trying to compile the uglfier assets in production mode in an environment with
dynatrace was causing conflicts with the execjs node runtime and the dynatrace
reporting mechanism.  We should be able to avoid this entirely if we only call
out to node for asset compilation when we do explicit asset compilation at build
time in a controlled environment without dynatrace instead of at runtime, which
we cannot control.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants