fix: truncated createReadStream through early end event (#2056)

* test: read entire file when range is `{start: 0}` * fix: `ReturnType<F>` regression * fix: Remove premature `.end()` calls
googleapis · Sep 1, 2022 · a4716a4 · a4716a4
1 parent c90ba0f
commit a4716a4
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 deletions.
diff --git a/src/file.ts b/src/file.ts
@@ -1417,9 +1417,7 @@ class File extends ServiceObject<File> {
     // Authenticate the request, then pipe the remote API request to the stream
     // returned to the user.
     const makeRequest = () => {
-      const query = {
-        alt: 'media',
-      } as FileQuery;
+      const query: FileQuery = {alt: 'media'};
 
       if (this.generation) {
         query.generation = this.generation;
@@ -1460,8 +1458,7 @@ class File extends ServiceObject<File> {
         })
         .on('response', res => {
           throughStream.emit('response', res);
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-          util.handleResp(null, res, null, onResponse as any);
+          util.handleResp(null, res, null, onResponse);
         })
         .resume();
 
@@ -1525,7 +1522,7 @@ class File extends ServiceObject<File> {
         const handoffStream = new PassThrough({
           final: async cb => {
             // Preserving `onComplete`'s ability to
-            // close `throughStream` before pipeline
+            // destroy `throughStream` before pipeline
             // attempts to.
             await onComplete(null);
             cb();
@@ -1558,7 +1555,6 @@ class File extends ServiceObject<File> {
         }
 
         if (rangeRequest || !shouldRunValidation) {
-          throughStream.end();
           return;
         }
 
@@ -1576,7 +1572,6 @@ class File extends ServiceObject<File> {
             return;
           }
           if (this.metadata.contentEncoding === 'gzip') {
-            throughStream.end();
             return;
           }
         }
@@ -1611,14 +1606,14 @@ class File extends ServiceObject<File> {
 
           throughStream.destroy(mismatchError);
         } else {
-          throughStream.end();
+          return;
         }
       };
     };
 
     throughStream.on('reading', makeRequest);
 
-    return throughStream as Readable;
+    return throughStream;
   }
 
   createResumableUpload(

diff --git a/system-test/storage.ts b/system-test/storage.ts
@@ -1719,7 +1719,7 @@ describe('storage', () => {
 
           // Validate the desired functionality
           const results = await testFunction(USER_PROJECT_OPTIONS);
-          return results;
+          return results as ReturnType<F>;
         }
 
         it('bucket#combine', async () => {
@@ -2119,6 +2119,14 @@ describe('storage', () => {
       assert.strictEqual(String(fileContents), String(remoteContents));
     });
 
+    it('should download an entire file if range `start:0` is provided', async () => {
+      const fileContents = fs.readFileSync(FILES.big.path);
+      const [file] = await bucket.upload(FILES.big.path);
+      const [result] = await file.download({start: 0});
+
+      assert.strictEqual(result.toString(), fileContents.toString());
+    });
+
     it('should download an empty file', async () => {
       const fileContents = fs.readFileSync(FILES.empty.path);
       const [file] = await bucket.upload(FILES.empty.path);