fix: don't always convert bytes to text in streamToPromise (#182)

* fix: don't always convert bytes to text in streamToPromise

Update doc in streamToPromise to indicate it is not always text.
Remove `toString()` call when using `Buffer` type.
Add test for binary type stream.
Update assertions for `Buffer` streams to assert length and checksums.

* fix: avoid redundant Buffer copy

lib/request-wrapper.ts

@@ -442,7 +442,8 @@ export class RequestWrapper {
 
     try {
       if (isStream(data)) {
-        reqBuffer = Buffer.from(await streamToPromise(data));
+        const streamData = await streamToPromise(data);
+        reqBuffer = Buffer.isBuffer(streamData) ? streamData : Buffer.from(streamData);
       } else if (data.toString && data.toString() !== '[object Object]' && !Array.isArray(data)) {
         // this handles pretty much any primitive that isnt a JSON object or array
         reqBuffer = Buffer.from(data.toString());

lib/stream-to-promise.ts

@@ -17,7 +17,8 @@
 import { Stream } from 'stream';
 
 /**
- * Helper method that can be bound to a stream - it sets the output to utf-8, captures all of the results, and returns a promise that resolves to the final text
+ * Helper method that can be bound to a stream - it captures all of the results, and returns a promise that resolves to the final buffer
+ * or array of text chunks
  * Essentially a smaller version of concat-stream wrapped in a promise
  *
  * @param {Stream} stream Optional stream param for when not bound to an existing stream instance.
@@ -32,7 +33,7 @@ export function streamToPromise(stream: Stream): Promise<any> {
         results.push(result);
       })
       .on('end', () => {
-        resolve(Buffer.isBuffer(results[0]) ? Buffer.concat(results).toString() : results);
+        resolve(Buffer.isBuffer(results[0]) ? Buffer.concat(results) : results);
       })
       .on('error', reject);
   });

test/unit/to-promise.test.js

@@ -14,17 +14,60 @@
  * limitations under the License.
  */
 
+const crypto = require('crypto');
 const fs = require('fs');
 const path = require('path');
+const { Transform } = require('stream');
 const toPromise = require('../../dist/lib/stream-to-promise').streamToPromise;
 
 describe('toPromise()', () => {
-  it('should resolve with results buffer as a string', () => {
+  // Testing stream of text in Buffer mode
+  it('should resolve with results buffer (text)', () => {
+    const fileChecksum = crypto.createHash('md5');
     const file = fs.createReadStream(path.join(__dirname, '../resources/weather-data-train.csv'));
-    // jest doesn't support type matching yet https://github.com/facebook/jest/issues/3457
-    return expect(toPromise(file).then((res) => typeof res)).resolves.toBe('string');
+    const p = toPromise(file);
+    file.pipe(fileChecksum);
+    return p
+      .then((buffer) => {
+        // Check we received a buffer
+        expect(buffer).toBeInstanceOf(Buffer);
+        // Check it was of the correct length
+        expect(buffer).toHaveLength(1794);
+        // Calculate a checksum
+        const promiseChecksum = crypto.createHash('md5');
+        promiseChecksum.update(buffer);
+        return promiseChecksum;
+      })
+      .then((promiseChecksum) => {
+        // Verify the checksum from the promise buffer matches the original file
+        expect(promiseChecksum.digest('hex')).toEqual(fileChecksum.digest('hex'));
+      });
   });
 
+  // Testing stream of text in Buffer mode
+  it('should resolve with results buffer (binary)', () => {
+    const fileChecksum = crypto.createHash('md5');
+    const file = fs.createReadStream(path.join(__dirname, '../resources/blank.wav'));
+    const p = toPromise(file);
+    file.pipe(fileChecksum);
+    return p
+      .then((buffer) => {
+        // Check we received a buffer
+        expect(buffer).toBeInstanceOf(Buffer);
+        // Check it was of the correct length
+        expect(buffer).toHaveLength(113520);
+        // Calculate a checksum
+        const promiseChecksum = crypto.createHash('md5');
+        promiseChecksum.update(buffer);
+        return promiseChecksum;
+      })
+      .then((promiseChecksum) => {
+        // Verify the checksum from the promise buffer matches the original file
+        expect(promiseChecksum.digest('hex')).toEqual(fileChecksum.digest('hex'));
+      });
+  });
+
+  // Testing stream of text in String mode
   it('should resolve with results string as an array', () => {
     const file = fs.createReadStream(path.join(__dirname, '../resources/weather-data-train.csv'));
     file.setEncoding('utf-8');