Node 12.16.3 broke grpc precompiled native addon installation on Windows

[murgatroid99]

• Version: 12.16.3
• Platform: Linux DESKTOP-OKC3QBQ 4.4.0-18362-Microsoft doc: fixed punctuation #476-Microsoft Fri Nov 01 16:53:00 PST 2019 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem:

What steps will reproduce the bug?

npm install grpc --verbose

How often does it reproduce? Is there a required condition?

It's consistent. I have only seen it happen on Windows.

What is the expected behavior?

When running that command on Node 12.16.2, I see these log lines:

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp info check checked for "/tmp/node_modules/grpc/src/node/extension_binary/node-v72-linux-x64-glibc/grpc_node.node" (not found)
node-pre-gyp http GET https://node-precompiled-binaries.grpc.io/grpc/v1.24.2/node-v72-linux-x64-glibc.tar.gz
node-pre-gyp http 200 https://node-precompiled-binaries.grpc.io/grpc/v1.24.2/node-v72-linux-x64-glibc.tar.gz
node-pre-gyp info install unpacking node-v72-linux-x64-glibc/grpc_node.node
node-pre-gyp info tarball done parsing tarball

What do you see instead?

When I run the same command on Node 12.16.3, I see these log lines:

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp info check checked for "/tmp/node_modules/grpc/src/node/extension_binary/node-v72-linux-x64-glibc/grpc_node.node" (not found)
node-pre-gyp http GET https://node-precompiled-binaries.grpc.io/grpc/v1.24.2/node-v72-linux-x64-glibc.tar.gz
node-pre-gyp http 200 https://node-precompiled-binaries.grpc.io/grpc/v1.24.2/node-v72-linux-x64-glibc.tar.gz
node-pre-gyp info install unpacking node-v72-linux-x64-glibc/grpc_node.node
node-pre-gyp WARN Pre-built binaries not installable for grpc@1.24.2 and node@12.16.3 (node-v72 ABI, glibc) (falling back to source compile with node-gyp)
node-pre-gyp WARN Hit error bad download

Additional information

[bcoe]

@richardlau closing the loop here, it looks like this is potentially related to a regression between 12.6.2 and 12.6.3 (probably the update to openssl as I believe you suggested).

Could it be this actually?

[cclauss]

https://docs.python.org/2/library/fpformat.html Removed in Python 3. Do you have a test case where the old and new disagree?

[bcoe]

@cclauss I'm working on a git bisect right now ... it seems unlikely that changing formatting would be the culprit, I will come back with more information.

[richardlau]

@richardlau closing the loop here, it looks like this is potentially related to a regression between 12.6.2 and 12.6.3 (probably the update to openssl as I believe you suggested).

Could it be this actually?

Unlikely as tools/configure.d/nodedownload.py only affects downloading ICU during the configure step of building Node.js itself (and isn't used in the released builds as they use the version of ICU in deps/.

[targos]

Do you know why node-pre-gyp thinks that the prebuilt binaries are not installable?

[cclauss]

$ python2

>>> import fpformat
>>> for amt in range(-1124000, 1124000):
...   try:
...     assert fpformat.fix(amt / 1024000., 1) == "%.1f" % (amt / 1024000.), amt
...   except AssertionError:
...     print(amt, fpformat.fix(amt / 1024000., 1), "%.1f" % (amt / 1024000.))
...
(-972800, '-1.0', '-0.9')
(-870400, '-0.9', '-0.8')
(-358400, '-0.4', '-0.3')
(-256000, '-0.3', '-0.2')
(-153600, '-0.2', '-0.1')
(153600, '0.2', '0.1')
(256000, '0.3', '0.2')
(358400, '0.4', '0.3')
(870400, '0.9', '0.8')
(972800, '1.0', '0.9')

[murgatroid99]

Looking at the source code of node-pre-gyp, it looks like you get that error when tarball download is interrupted partway through with a network or HTTP error.

[murgatroid99]

If I install request before installing grpc, so that node-pre-gyp uses request instead of needle, I don't see this failure. So this is a network issue, and it's related to something needle is doing differently from request.

[bcoe]

👋 having bisected v12.16.2 ... v12.16.3, it looks like the culprit is this:

ed21d32

Any thoughts @ronag @targos, my concern is that we might be breaking a variety of other Windows builds.

~edit, I was wrong about which specific part of the patch had issues._

[ronag]

Is this a problem on v14 as well? I assume OSX and Linux can't reproduce it? If you build it on VS2017 and/or VS2019 do you have the same problem?

@bcoe: Hm, I'm sceptical whether that change in itself is the direct cause, though it might have indirectly triggered another bug.

[bcoe]

@ronag it does crop up in v14 as well. I am able to reproduce on my Linux work computer:

with ed21d32a7c37d030ce4668415832a89dc8ead34c:

bencoe@bencoe:~/oss/node$ /usr/local/google/home/bencoe/.nvm/versions/node/v10.15.3/bin/npm i grpc

> grpc@1.24.2 install /usr/local/google/home/bencoe/oss/node/node_modules/grpc
> node-pre-gyp install --fallback-to-build --library=static_library

node-pre-gyp WARN Using needle for node-pre-gyp https download 
node-pre-gyp WARN Pre-built binaries not installable for grpc@1.24.2 and node@12.16.3-pre (node-v72 ABI, glibc) (falling back to source compile with node-gyp) 
node-pre-gyp WARN Hit error bad download 

resetting to 959055f2258828c42273904f58c52e685a833539:

bencoe@bencoe:~/oss/node$ /usr/local/google/home/bencoe/.nvm/versions/node/v10.15.3/bin/npm i grpc

> grpc@1.24.2 install /usr/local/google/home/bencoe/oss/node/node_modules/grpc
> node-pre-gyp install --fallback-to-build --library=static_library

node-pre-gyp WARN Using needle for node-pre-gyp https download 
[grpc] Success: "/usr/local/google/home/bencoe/oss/node/node_modules/grpc/src/node/extension_binary/node-v72-linux-x64-glibc/grpc_node.node" is installed via remote

[bcoe]

clarification, @murgatroid99 points out that grpc does not have precompiled binaries for Node 14, so the failing Windows tests on 14 would be unrelated.

[ronag]

What exactly does:

Pre-built binaries not installable for grpc@1.24.2 and node@12.16.3-pre (node-v72 ABI, glibc) (falling back to source compile with node-gyp)

Mean? Isn't this just a case of pre-built binaries missing?

[ronag]

Building in docker, but will take a while. Will try to get back to this tomorrow evening.

[bcoe]

@ronag for what it's worth, if I remove the -pre in the version I'm compiling:

> grpc@1.24.2 install /usr/local/google/home/bencoe/oss/node_modules/grpc
> node-pre-gyp install --fallback-to-build --library=static_library

node-pre-gyp WARN Using needle for node-pre-gyp https download 
node-pre-gyp WARN Pre-built binaries not installable for grpc@1.24.2 and node@12.16.3 (node-v72 ABI, glibc) (falling back to source compile with node-gyp) 
node-pre-gyp WARN Hit error bad download 
make: Entering directory '/usr/local/google/home/bencoe/oss/node_modules/grpc/build'
  CXX(target) Release/obj.target/grpc/deps/grpc/src/core/lib/surface/init.o
  CXX(target) Release/obj.target/grpc/deps/grpc/src/core/lib/avl/avl.o
  CXX(target) Release/obj.target/grpc/deps/grpc/src/core/lib/backoff/backoff.o
  CXX(target) Release/obj.target/grpc/deps/grpc/src/core/lib/channel/channel_args.o

☝️ I still get the "error bad download", and rather than downloading the binary it falls back to compiling (which I think wouldn't happen if the binary had come down okay).

If I rollback to 959055f, I get the download and no compile is necessary.

---

I think what's happening is that we fail to download the binary on all platforms, but then the fallback to compiling works on Linux, on Windows grpc does not compile, so by missing the binary download we fail to install grpc.

[ronag]

Is it possible to get the actual error from needle?

[ronag]

This seems to have something to do with needle. Do you think you could create a simple repro using needle? Based on what node-pre-gyp does https://github.com/mapbox/node-pre-gyp/blob/590da5603bb77df9583bfdc54d05d2267cadb364/lib/install.js.

I suspect we hit this path for some reason after 'response'.

[ronag]

This looks wrong to me in needle.

The socket is not destroyed even if 'end' is emitted and it is no longer writable, until it is flushed.

[ronag]

Yes, tomas/needle#312. It looks like it will be sorted in needle.

[bcoe]

@ronag awesome, I'm glad we tracked it down. Sorry if I flagged you in error (and appreciate the help digging).

[richardlau]

This sounds like it was fixed in needle.