Repository: ekalinin/nodeenv

Issue:
download_node_src does not properly handle multipart downloads
The method ``download_node_src`` fails if the download doesn't complete in a single part. In my case this lead to a seemingly unrelated error ``AttributeError: 'bytes' object has no attribute 'tell'`` deep in the tarfile module. The exception handler,

```
    try:
        dl_contents = io.BytesIO(urlopen(node_url).read())
    except IncompleteRead as e:
        logger.warning('Incomplete read while reading'
                       'from {}'.format(node_url))
        dl_contents = e.partial
```

assigned a bytes object to ``dl_contents`` instead of a ``BytesIO``. However, updating the exception still did not work because "partial" really does mean partial, and is not the complete file so there is no way to use this. Also, simply calling read() again also appears to be the way to handle multipart.

I got this to work by using requests, which appears to handle this properly

```
    import requests
    dl_contents = io.BytesIO(requests.get(node_url).content)
```



Discussion:
bagerard: Done fixing the issues @ekalinin , please approve the workflow when you have a chance
bagerard: ping @ekalinin, this is really impacful for our pipelines so it'd be great if it could move forward
c3b5aw: Bump, this is happening through usage of https://github.com/RobertCraigie/prisma-client-py as well.
Alexander-Serov: @ekalinin Could you please review it? This is an important fix for us.

Patch:
diff --git a/nodeenv.py b/nodeenv.py
index 53ff5d7..f9c1d9e 100644
--- a/nodeenv.py
+++ b/nodeenv.py
@@ -582,17 +582,28 @@ def tarfile_open(*args, **kwargs):
         tf.close()
 
 
+def _download_node_file(node_url, n_attempt=3):
+    """Do multiple attempts to avoid incomplete data in case
+    of unstable network"""
+    while n_attempt > 0:
+        try:
+            return io.BytesIO(urlopen(node_url).read())
+        except IncompleteRead as e:
+            logger.warning(
+                'Incomplete read while reading'
+                'from {} - {}'.format(node_url, e)
+            )
+            n_attempt -= 1
+            if n_attempt == 0:
+                raise e
+
+
 def download_node_src(node_url, src_dir, args):
     """
     Download source code
     """
     logger.info('.', extra=dict(continued=True))
-    try:
-        dl_contents = io.BytesIO(urlopen(node_url).read())
-    except IncompleteRead as e:
-        logger.warning('Incomplete read while reading'
-                       'from {}'.format(node_url))
-        dl_contents = e.partial
+    dl_contents = _download_node_file(node_url)
     logger.info('.', extra=dict(continued=True))
 
     if is_WIN or is_CYGWIN:


Test patch:
diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py
index ab59775..09bcbb0 100644
--- a/tests/nodeenv_test.py
+++ b/tests/nodeenv_test.py
@@ -15,7 +15,7 @@ except ImportError:
 import pytest
 
 import nodeenv
-
+from nodeenv import IncompleteRead
 
 HERE = os.path.abspath(os.path.dirname(__file__))
 
@@ -146,3 +146,14 @@ def test_get_latest_node_version():
 @pytest.mark.usefixtures('mock_index_json')
 def test_get_lts_node_version():
     assert nodeenv.get_last_lts_node_version() == '12.14.0'
+
+
+def test__download_node_file():
+    with mock.patch.object(nodeenv, 'urlopen') as m_urlopen:
+        m_urlopen.side_effect = IncompleteRead("dummy")
+        with pytest.raises(IncompleteRead):
+            nodeenv._download_node_file(
+                "https://dummy/nodejs.tar.gz",
+                n_attempt=5
+            )
+        assert m_urlopen.call_count == 5