Skip to content

Node 12.16.3 broke grpc precompiled native addon installation on Windows #33254

Closed
Closed
Node 12.16.3 broke grpc precompiled native addon installation on Windows#33254
Labels
netIssues and PRs related to the net subsystem.

Description

@murgatroid99
murgatroid99
opened on May 5, 2020
  • 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

Activity

  1. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

    @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?

  2. cclauss commented on May 5, 2020

    @cclauss
    cclauss
    on May 5, 2020
    Contributor

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

  3. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

    @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.

  4. richardlau commented on May 5, 2020

    @richardlau
    richardlau
    on May 5, 2020
    Member

    @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/.

  5. targos commented on May 5, 2020

    @targos
    targos
    on May 5, 2020
    Member

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

  6. cclauss commented on May 5, 2020

    @cclauss
    cclauss
    on May 5, 2020
    Contributor

    $ 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')

  7. murgatroid99 commented on May 5, 2020

    @murgatroid99
    murgatroid99
    on May 5, 2020
    ContributorAuthor

    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.

  8. murgatroid99 commented on May 5, 2020

    @murgatroid99
    murgatroid99
    on May 5, 2020
    ContributorAuthor

    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.

  9. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

    👋 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._

  10. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

    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.

  11. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

    @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

  12. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

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

  13. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

    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?

  14. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

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

  15. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

    @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.

  16. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

    Is it possible to get the actual error from needle?

  17. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

    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'.

  18. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

    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.

  19. ronag commented on May 5, 2020

    @ronag
    ronag
    on May 5, 2020
    Member

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

  20. ronag
    added
    netIssues and PRs related to the net subsystem.
    on May 5, 2020

  21. bcoe commented on May 5, 2020

    @bcoe
    bcoe
    on May 5, 2020
    Contributor

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

  22. kintel
    added a commit that references this issue on May 6, 2020

    Change Windows CI to use Node.js 10

    a242700
  23. bcoe
    added a commit that references this issue on May 6, 2020

    build: switch to Node 10 for windows CI (#1258)

    ac1f484

  24. richardlau commented on Feb 17, 2022

    @richardlau
    richardlau
    on Feb 17, 2022
    Member

    This sounds like it was fixed in needle.

  25. richardlau
    closed this as completedon Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    netIssues and PRs related to the net subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions