fix: improve file upload handling in prepare-request to use streaming (#5637)

* fix: improve file upload handling in prepare-request to use streaming
* feat: add unit tests

---------

Co-authored-by: Bijin Bruno <bijin@usebruno.com>

packages/bruno-cli/src/runner/prepare-request.js

@@ -4,10 +4,13 @@ const filter = require('lodash/filter');
 const find = require('lodash/find');
 const decomment = require('decomment');
 const crypto = require('node:crypto');
-const fs = require('node:fs/promises');
+const fs = require('node:fs');
 const { mergeHeaders, mergeScripts, mergeVars, mergeAuth, getTreePathFromCollectionToItem } = require('../utils/collection');
 const { buildFormUrlEncodedPayload } = require('../utils/form-data');
 const path = require('node:path');
+const { isLargeFile } = require('../utils/filesystem');
+
+const STREAMING_FILE_SIZE_THRESHOLD = 20 * 1024 * 1024; // 20MB
 
 const prepareRequest = async (item = {}, collection = {}) => {
   const request = item?.request;
@@ -311,8 +314,14 @@ const prepareRequest = async (item = {}, collection = {}) => {
         }
 
         try {
-          const fileContent = await fs.readFile(filePath);
-          axiosRequest.data = fileContent;
+          // Large files can cause "JavaScript heap out of memory" errors when loaded entirely into memory.
+          if (isLargeFile(filePath, STREAMING_FILE_SIZE_THRESHOLD)) {
+            // For large files: Use streaming to avoid memory issues
+            axiosRequest.data = fs.createReadStream(filePath);
+          } else {
+            // For smaller files: Use synchronous read for better performance
+            axiosRequest.data = fs.readFileSync(filePath);
+          }
         } catch (error) {
           console.error('Error reading file:', error);
         }

packages/bruno-cli/src/utils/filesystem.js

@@ -158,6 +158,22 @@ const validateName = (name) => {
   );
 };
 
+/**
+ * Checks if a file is larger than a given threshold.
+ * @param {string} filePath - The path to the file.
+ * @param {number} threshold - The threshold in bytes. Default is 10MB.
+ * @returns {boolean} True if the file is larger than the threshold, false otherwise.
+ */
+const isLargeFile = (filePath, threshold = 10 * 1024 * 1024) => {
+  if (!isFile(filePath)) {
+    throw new Error(`File ${filePath} is not a file`);
+  }
+
+  const size = fs.statSync(filePath).size;
+
+  return size > threshold;
+};
+
 module.exports = {
   exists,
   isSymbolicLink,
@@ -173,5 +189,6 @@ module.exports = {
   stripExtension,
   getSubDirectories,
   sanitizeName,
-  validateName
+  validateName,
+  isLargeFile
 };

packages/bruno-cli/tests/runner/prepare-request.spec.js

@@ -1,4 +1,8 @@
 const { describe, it, expect, beforeEach } = require('@jest/globals');
+jest.mock('../../src/utils/filesystem', () => ({
+  isLargeFile: jest.fn()
+}));
+const filesystemUtils = require('../../src/utils/filesystem');
 const prepareRequest = require('../../src/runner/prepare-request');
 
 describe('prepare-request: prepareRequest', () => {
@@ -521,21 +525,23 @@ describe('prepare-request: prepareRequest', () => {
   });
 
   describe('Request file body mode', () => {
-    it('reads the uploaded file and applies correct headers', async () => {
-      const fsPromises = require('node:fs/promises');
-      // Mock fs.readFile to avoid actual file system dependency
-      jest.spyOn(fsPromises, 'readFile').mockResolvedValue(Buffer.from('dummy file content'));
+    const fs = require('node:fs');
+    let readFileSyncSpy;
+    let createReadStreamSpy;
 
-      const body = {
-        mode: 'file',
-        file: [
-          {
-            contentType: 'text/plain',
-            filePath: '/absolute/path/to/file.txt',
-            selected: true,
-          },
-        ],
-      };
+    beforeEach(() => {
+      readFileSyncSpy = jest.spyOn(fs, 'readFileSync');
+      createReadStreamSpy = jest.spyOn(fs, 'createReadStream');
+    });
+
+    afterEach(() => {
+      jest.restoreAllMocks();
+    });
+
+    it('should use readFileSync to read small files', async () => {
+      const fileContent = Buffer.from('small file content');
+      filesystemUtils.isLargeFile.mockReturnValue(false);
+      readFileSyncSpy.mockReturnValue(fileContent);
 
       const item = {
         name: 'File Request',
@@ -545,13 +551,53 @@ describe('prepare-request: prepareRequest', () => {
           headers: [],
           params: [],
           url: 'https://example.com/upload',
-          body,
+          body: {
+            mode: 'file',
+            file: [{
+              contentType: 'text/plain',
+              filePath: '/path/to/file.txt',
+              selected: true
+            }]
+          }
         },
       };
 
       const result = await prepareRequest(item);
-      expect(result.data).toBeInstanceOf(Buffer);
-      expect(result.headers['content-type']).toBe('text/plain');
+
+      expect(result.data).toBe(fileContent);
+      expect(readFileSyncSpy).toHaveBeenCalled();
+      expect(createReadStreamSpy).not.toHaveBeenCalled();
+    });
+
+    it('should use createReadStream to read large files', async () => {
+      const mockStream = { pipe: jest.fn() };
+      filesystemUtils.isLargeFile.mockReturnValue(true);
+      createReadStreamSpy.mockReturnValue(mockStream);
+
+      const item = {
+        name: 'File Request',
+        type: 'http-request',
+        request: {
+          method: 'POST',
+          headers: [],
+          params: [],
+          url: 'https://example.com/upload',
+          body: {
+            mode: 'file',
+            file: [{
+              contentType: 'application/octet-stream',
+              filePath: '/path/to/large-file.bin',
+              selected: true
+            }]
+          }
+        }
+      };
+
+      const result = await prepareRequest(item);
+
+      expect(result.data).toBe(mockStream);
+      expect(createReadStreamSpy).toHaveBeenCalled();
+      expect(readFileSyncSpy).not.toHaveBeenCalled();
     });
   });
 });

packages/bruno-cli/tests/utils/filesystem.spec.js

@@ -0,0 +1,50 @@
+const { isLargeFile } = require('../../src/utils/filesystem');
+const fs = require('fs-extra');
+
+describe('isLargeFile', () => {
+  let existsSyncSpy;
+  let lstatSyncSpy;
+  let statSyncSpy;
+
+  beforeEach(() => {
+    existsSyncSpy = jest.spyOn(fs, 'existsSync');
+    lstatSyncSpy = jest.spyOn(fs, 'lstatSync');
+    statSyncSpy = jest.spyOn(fs, 'statSync');
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('should return false when file size is below default threshold (10MB)', () => {
+    existsSyncSpy.mockReturnValue(true);
+    lstatSyncSpy.mockReturnValue({ isFile: () => true });
+    statSyncSpy.mockReturnValue({ size: 5 * 1024 * 1024 }); // 5MB
+
+    expect(isLargeFile('/path/small.bin')).toBe(false);
+  });
+
+  it('should return true when file size is above default threshold (10MB)', () => {
+    existsSyncSpy.mockReturnValue(true);
+    lstatSyncSpy.mockReturnValue({ isFile: () => true });
+    statSyncSpy.mockReturnValue({ size: 15 * 1024 * 1024 }); // 15MB
+
+    expect(isLargeFile('/path/large.bin')).toBe(true);
+  });
+
+  it('should respect custom threshold (args true or false)', () => {
+    existsSyncSpy.mockReturnValue(true);
+    lstatSyncSpy.mockReturnValue({ isFile: () => true });
+    statSyncSpy.mockReturnValue({ size: 50 });
+
+    expect(isLargeFile('/path/file.bin', 100)).toBe(false); // 50 < 100
+    expect(isLargeFile('/path/file.bin', 10)).toBe(true); // 50 > 10
+  });
+
+  it('should throw on invalid values (not a file)', () => {
+    existsSyncSpy.mockReturnValue(false);
+    lstatSyncSpy.mockReturnValue({ isFile: () => false });
+
+    expect(() => isLargeFile('/path/not-a-file.bin')).toThrow('File /path/not-a-file.bin is not a file');
+  });
+});

packages/bruno-electron/src/ipc/network/prepare-request.js

@@ -1,10 +1,13 @@
 const { get, each, filter, find } = require('lodash');
 const decomment = require('decomment');
 const crypto = require('node:crypto');
-const fs = require('node:fs/promises');
+const fs = require('node:fs');
 const { getTreePathFromCollectionToItem, mergeHeaders, mergeScripts, mergeVars, getFormattedCollectionOauth2Credentials, mergeAuth } = require('../../utils/collection');
 const { buildFormUrlEncodedPayload } = require('../../utils/form-data');
 const path = require('node:path');
+const { isLargeFile } = require('../../utils/filesystem');
+
+const STREAMING_FILE_SIZE_THRESHOLD = 20 * 1024 * 1024; // 20MB
 
 const setAuthHeaders = (axiosRequest, request, collectionRoot) => {
   const collectionAuth = get(collectionRoot, 'request.auth');
@@ -398,8 +401,14 @@ const prepareRequest = async (item, collection = {}, abortController) => {
         }
   
         try {
-          const fileContent = await fs.readFile(filePath);
-          axiosRequest.data = fileContent;
+          // Large files can cause "JavaScript heap out of memory" errors when loaded entirely into memory.
+          if (isLargeFile(filePath, STREAMING_FILE_SIZE_THRESHOLD)) {
+            // For large files: Use streaming to avoid memory issues
+            axiosRequest.data = fs.createReadStream(filePath);
+          } else {
+            // For smaller files: Use synchronous read for better performance
+            axiosRequest.data = fs.readFileSync(filePath);
+          }
         } catch (error) {
           console.error('Error reading file:', error);
         }

packages/bruno-electron/src/utils/filesystem.js

@@ -343,6 +343,21 @@ const getPaths = async (source) => {
   return paths;
 }
 
+/**
+ * Checks if a file is larger than a given threshold.
+ * @param {string} filePath - The path to the file.
+ * @param {number} threshold - The threshold in bytes. Default is 10MB.
+ * @returns {boolean} True if the file is larger than the threshold, false otherwise.
+ */
+const isLargeFile = (filePath, threshold = 10 * 1024 * 1024) => {
+  if (!isFile(filePath)) {
+    throw new Error(`File ${filePath} is not a file`);
+  }
+
+  const size = fs.statSync(filePath).size;
+
+  return size > threshold;
+};
 
 module.exports = {
   isValidPathname,
@@ -373,5 +388,6 @@ module.exports = {
   safeWriteFileSync,
   copyPath,
   removePath,
-  getPaths
+  getPaths,
+  isLargeFile
 };

packages/bruno-electron/src/utils/filesystem.test.js

@@ -1,4 +1,5 @@
-const { sanitizeName, isWSLPath, normalizeWSLPath, normalizeAndResolvePath } = require('./filesystem.js');
+const { sanitizeName, isWSLPath, normalizeWSLPath, normalizeAndResolvePath, isLargeFile } = require('./filesystem.js');
+const fs = require('fs-extra');
 
 describe('sanitizeName', () => {
   it('should replace invalid characters with hyphens', () => {
@@ -29,6 +30,54 @@ describe('sanitizeName', () => {
   });
 });
 
+describe('isLargeFile', () => {
+  let existsSyncSpy;
+  let lstatSyncSpy;
+  let statSyncSpy;
+
+  beforeEach(() => {
+    existsSyncSpy = jest.spyOn(fs, 'existsSync');
+    lstatSyncSpy = jest.spyOn(fs, 'lstatSync');
+    statSyncSpy = jest.spyOn(fs, 'statSync');
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('should return false when file size is below default threshold (10MB)', () => {
+    existsSyncSpy.mockReturnValue(true);
+    lstatSyncSpy.mockReturnValue({ isFile: () => true });
+    statSyncSpy.mockReturnValue({ size: 5 * 1024 * 1024 }); // 5MB
+
+    expect(isLargeFile('/path/small.bin')).toBe(false);
+  });
+
+  it('should return true when file size is above default threshold (10MB)', () => {
+    existsSyncSpy.mockReturnValue(true);
+    lstatSyncSpy.mockReturnValue({ isFile: () => true });
+    statSyncSpy.mockReturnValue({ size: 15 * 1024 * 1024 }); // 15MB
+
+    expect(isLargeFile('/path/large.bin')).toBe(true);
+  });
+
+  it('should respect custom threshold (args true or false)', () => {
+    existsSyncSpy.mockReturnValue(true);
+    lstatSyncSpy.mockReturnValue({ isFile: () => true });
+    statSyncSpy.mockReturnValue({ size: 50 });
+
+    expect(isLargeFile('/path/file.bin', 100)).toBe(false); // 50 < 100
+    expect(isLargeFile('/path/file.bin', 10)).toBe(true); // 50 > 10
+  });
+
+  it('should throw on invalid values (not a file)', () => {
+    existsSyncSpy.mockReturnValue(false);
+    lstatSyncSpy.mockReturnValue({ isFile: () => false });
+
+    expect(() => isLargeFile('/path/not-a-file.bin')).toThrow('File /path/not-a-file.bin is not a file');
+  });
+});
+
 describe('WSL Path Utilities', () => {
   describe('isWSLPath', () => {
     it('should identify WSL paths starting with double backslash', () => {