refactor: streamline hook management by removing unnecessary HookManager instances and optimizing hook execution flow

2026-06-26 22:25:40 +00:00 · 2026-01-22 13:30:26 +05:30
parent 777707180e
commit cc33299702
4 changed files with 257 additions and 464 deletions
--- a/packages/bruno-cli/src/commands/run.js
+++ b/packages/bruno-cli/src/commands/run.js
@@ -15,10 +15,11 @@ const { rpad } = require('../utils/common');
 const { getOptions } = require('../utils/bru');
 const { parseDotEnv, parseEnvironment } = require('@usebruno/filestore');
 const constants = require('../constants');
-const { findItemInCollection, createCollectionJsonFromPathname, getCallStack, FORMAT_CONFIG, HOOK_EVENTS, getOrCreateHookManager } = require('../utils/collection');
+const { findItemInCollection, createCollectionJsonFromPathname, getCallStack, FORMAT_CONFIG, HOOK_EVENTS } = require('../utils/collection');
 const { hasExecutableTestInScript } = require('../utils/request');
 const { createSkippedFileResults } = require('../utils/run');
-const HookManager = require('@usebruno/js/src/hook-manager');
+const { HooksRuntime } = require('@usebruno/js');
+const decomment = require('decomment');
 const command = 'run [paths...]';
 const desc = 'Run one or more requests/folders';

@@ -616,46 +617,49 @@ const handler = async function (argv) {
    const scriptingConfig = get(brunoConfig, 'scripts', {});
    scriptingConfig.runtime = runtime;

-    // Create HookManager map to share HookManagers across requests
-    const hookManagersMap = new Map();
    const collectionName = collection?.brunoConfig?.name;
    const onConsoleLog = (type, args) => {
      console[type](...args);
    };

-    // Register collection-level hooks once at the start
-    collectionRoot = collection?.draft?.root || collection?.root || {};
-    const collectionHooks = get(collectionRoot, 'request.script.hooks', '');
-    const collectionHookManagerKey = `collection:${collection.pathname}`;
-    let collectionHookManager = null;
+    // Helper function to execute collection-level hooks at runtime
+    const executeCollectionHooks = async (hookEvent, eventData) => {
+      collectionRoot = collection?.draft?.root || collection?.root || {};
+      const collectionHooks = get(collectionRoot, 'request.script.hooks', '');

-    if (collectionHooks && collectionHooks.trim()) {
-      const hookManagerOptions = {
-        request: {}, // Placeholder request for hook registration
-        envVariables: envVars,
-        runtimeVariables,
-        collectionPath,
-        onConsoleLog,
-        processEnvVars,
-        scriptingConfig,
-        runRequestByItemPathname: null, // Not available at collection level
-        collectionName
-      };
-      collectionHookManager = await getOrCreateHookManager(hookManagersMap, collectionHookManagerKey, collectionHooks, hookManagerOptions);
-    } else {
-      // Create empty HookManager for collection even if no hooks
-      collectionHookManager = new HookManager();
-      hookManagersMap.set(collectionHookManagerKey, collectionHookManager);
-    }
+      if (!collectionHooks || !collectionHooks.trim()) {
+        return;
+      }
+
+      try {
+        const hooksRuntime = new HooksRuntime({ runtime: scriptingConfig?.runtime });
+        const result = await hooksRuntime.runHooks({
+          hooksFile: decomment(collectionHooks),
+          request: {}, // Placeholder request for collection-level hooks
+          envVariables: envVars,
+          runtimeVariables,
+          collectionPath,
+          onConsoleLog,
+          processEnvVars,
+          scriptingConfig,
+          runRequestByItemPathname: null, // Not available at collection level
+          collectionName
+        });
+
+        if (result?.hookManager) {
+          await result.hookManager.call(hookEvent, eventData);
+          // Dispose HookManager to free VM resources
+          if (result.hookManager && typeof result.hookManager.dispose === 'function') {
+            result.hookManager.dispose();
+          }
+        }
+      } catch (error) {
+        console.error(`Error executing collection-level hooks for ${hookEvent}:`, error);
+      }
+    };

    // Call onBeforeCollectionRun hook before starting to run requests
-    if (collectionHookManager) {
-      try {
-        await collectionHookManager.call(HOOK_EVENTS.RUNNER_BEFORE_COLLECTION_RUN, { collection });
-      } catch (error) {
-        console.error('Error calling onBeforeCollectionRun hooks:', error);
-      }
-    }
+    await executeCollectionHooks(HOOK_EVENTS.RUNNER_BEFORE_COLLECTION_RUN, { collection });

    const runSingleRequestByPathname = async (relativeItemPathname) => {
      const ext = FORMAT_CONFIG[collection.format].ext;
@@ -676,8 +680,7 @@ const handler = async function (argv) {
            collectionRoot,
            runtime,
            collection,
-            runSingleRequestByPathname,
-            hookManagersMap
+            runSingleRequestByPathname
          );
          resolve(res?.response);
        }
@@ -702,8 +705,7 @@ const handler = async function (argv) {
        collectionRoot,
        runtime,
        collection,
-        runSingleRequestByPathname,
-        hookManagersMap
+        runSingleRequestByPathname
      );

      const isLastRun = currentRequestIndex === requestItems.length - 1;
@@ -799,26 +801,7 @@ const handler = async function (argv) {
    results.push(...skippedFileResults);

    // Call onAfterCollectionRun hook after all requests are done
-    if (collectionHookManager) {
-      try {
-        await collectionHookManager.call(HOOK_EVENTS.RUNNER_AFTER_COLLECTION_RUN, { collection });
-      } catch (error) {
-        console.error('Error calling onAfterCollectionRun hooks:', error);
-      }
-    }
-
-    // Cleanup: Dispose all HookManagers to free VM resources, then clear the map
-    // This is critical to prevent memory leaks from persisted QuickJS VMs
-    hookManagersMap.forEach((hookManager) => {
-      if (hookManager && typeof hookManager.dispose === 'function') {
-        try {
-          hookManager.dispose();
-        } catch (e) {
-          // Ignore disposal errors
-        }
-      }
-    });
-    hookManagersMap.clear();
+    await executeCollectionHooks(HOOK_EVENTS.RUNNER_AFTER_COLLECTION_RUN, { collection });

    const summary = printRunSummary(results);
    const runCompletionTime = new Date().toISOString();
--- a/packages/bruno-cli/src/runner/run-single-request.js
+++ b/packages/bruno-cli/src/runner/run-single-request.js
@@ -12,7 +12,7 @@ const BrunoRequest = require('@usebruno/js/src/bruno-request');
 const BrunoResponse = require('@usebruno/js/src/bruno-response');
 const { stripExtension } = require('../utils/filesystem');
 const { getOptions } = require('../utils/bru');
-const { extractHooks, getTreePathFromCollectionToItem, HOOK_EVENTS, getOrCreateHookManager } = require('../utils/collection');
+const { extractHooks, getTreePathFromCollectionToItem, HOOK_EVENTS } = require('../utils/collection');
 const https = require('https');
 const { HttpProxyAgent } = require('http-proxy-agent');
 const { SocksProxyAgent } = require('socks-proxy-agent');
@@ -97,8 +97,7 @@ const runSingleRequest = async function (
  collectionRoot,
  runtime,
  collection,
-  runSingleRequestByPathname,
-  hookManagersMap
+  runSingleRequestByPathname
 ) {
  const { pathname: itemPathname } = item;
  const relativeItemPathname = path.relative(collectionPath, itemPathname);
@@ -173,15 +172,21 @@ const runSingleRequest = async function (
    const requestTreePath = getTreePathFromCollectionToItem(collection, item);
    const collectionName = collection?.brunoConfig?.name;

-    // Get or create HookManagers for each level using shared map
-    let allHookManagers = [];
-    if (hookManagersMap) {
-      try {
-        const { collectionHooks, folderHooks, requestHooks } = extractHooks(collection, request, requestTreePath);
+    // Extract hooks for all levels
+    const { collectionHooks, folderHooks, requestHooks } = extractHooks(collection, request, requestTreePath);

-        const hookManagerOptions = {
+    // Helper function to initialize and execute hooks at runtime
+    const executeHooksForLevel = async (hooksFile, hookEvent, eventData) => {
+      if (!hooksFile || !hooksFile.trim()) {
+        return;
+      }
+
+      try {
+        const hooksRuntime = new HooksRuntime({ runtime: scriptingConfig?.runtime });
+        const result = await hooksRuntime.runHooks({
+          hooksFile: decomment(hooksFile),
          request,
-          envVars: envVariables, // Will be mapped to envVariables in runHooks
+          envVariables,
          runtimeVariables,
          collectionPath,
          onConsoleLog,
@@ -189,43 +194,35 @@ const runSingleRequest = async function (
          scriptingConfig,
          runRequestByItemPathname: runSingleRequestByPathname,
          collectionName
-        };
+        });

-        // Collection-level HookManager (shared across all requests)
-        const collectionHookManagerKey = `collection:${collection.pathname}`;
-        const collectionHookManager = await getOrCreateHookManager(hookManagersMap, collectionHookManagerKey, collectionHooks, hookManagerOptions);
-
-        // Folder-level HookManagers (in order from collection to request)
-        const folderHookManagers = [];
-        for (const folderHook of folderHooks) {
-          // folderPathname is set by extractHooks (i.pathname)
-          const folderHookManagerKey = `folder:${folderHook.folderPathname}`;
-          const folderHookManager = await getOrCreateHookManager(hookManagersMap, folderHookManagerKey, folderHook.hooks, hookManagerOptions);
-          folderHookManagers.push(folderHookManager);
+        if (result?.hookManager) {
+          await result.hookManager.call(hookEvent, eventData);
+          // Dispose HookManager to free VM resources
+          if (result.hookManager && typeof result.hookManager.dispose === 'function') {
+            result.hookManager.dispose();
+          }
        }
-
-        // Request-level HookManager (unique per request)
-        const requestHookManagerKey = item.pathname;
-        const requestHookManager = await getOrCreateHookManager(hookManagersMap, requestHookManagerKey, requestHooks, hookManagerOptions);
-
-        // Combine all HookManagers in order: collection -> folder(s) -> request
-        allHookManagers = [collectionHookManager, ...folderHookManagers, requestHookManager];
      } catch (error) {
-        console.error('Error getting/creating hook managers:', error);
+        console.error(`Error executing hooks for ${hookEvent}:`, error);
      }
-    }
+    };

    // Call beforeRequest hooks before running pre-request scripts
    // Hooks are called in registration order: collection -> folder(s) -> request
-    for (const hookManager of allHookManagers) {
-      try {
-        const req = new BrunoRequest(request);
-        await hookManager.call(HOOK_EVENTS.HTTP_BEFORE_REQUEST, { request, req, collection });
-      } catch (error) {
-        console.error('Error calling beforeRequest hooks:', error);
-      }
+    const beforeRequestEventData = { request, req: new BrunoRequest(request), collection };
+
+    // Collection-level beforeRequest hooks
+    await executeHooksForLevel(collectionHooks, HOOK_EVENTS.HTTP_BEFORE_REQUEST, beforeRequestEventData);
+
+    // Folder-level beforeRequest hooks (in order from collection to request)
+    for (const folderHook of folderHooks) {
+      await executeHooksForLevel(folderHook.hooks, HOOK_EVENTS.HTTP_BEFORE_REQUEST, beforeRequestEventData);
    }

+    // Request-level beforeRequest hooks
+    await executeHooksForLevel(requestHooks, HOOK_EVENTS.HTTP_BEFORE_REQUEST, beforeRequestEventData);
+
    // run pre request script
    const requestScriptFile = get(request, 'script.req');
    if (requestScriptFile?.length) {
@@ -251,11 +248,6 @@ const runSingleRequest = async function (
      }

      if (result?.skipRequest) {
-        // Clean up request-level hook manager if request is skipped
-        if (hookManagersMap && allHookManagers.length > 0) {
-          const requestHookManagerKey = item.pathname;
-          hookManagersMap.delete(requestHookManagerKey);
-        }
        return {
          test: {
            filename: relativeItemPathname
@@ -708,23 +700,24 @@ const runSingleRequest = async function (

    // Call afterResponse hooks after response is received but before post-response scripts
    // Hooks are called in registration order: collection -> folder(s) -> request
-    for (const hookManager of allHookManagers) {
-      try {
-        const req = new BrunoRequest(request);
-        const res = new BrunoResponse(response);
-        await hookManager.call(HOOK_EVENTS.HTTP_AFTER_RESPONSE, { request, response, req, res, collection });
-      } catch (error) {
-        console.error('Error calling afterResponse hooks:', error);
-      }
+    const afterResponseEventData = {
+      request,
+      response,
+      req: new BrunoRequest(request),
+      res: new BrunoResponse(response),
+      collection
+    };
+
+    // Collection-level afterResponse hooks
+    await executeHooksForLevel(collectionHooks, HOOK_EVENTS.HTTP_AFTER_RESPONSE, afterResponseEventData);
+
+    // Folder-level afterResponse hooks (in order from collection to request)
+    for (const folderHook of folderHooks) {
+      await executeHooksForLevel(folderHook.hooks, HOOK_EVENTS.HTTP_AFTER_RESPONSE, afterResponseEventData);
    }

-    // Clean up request-level hook manager after request completes
-    // Requests are only run once, so we can safely remove the hook manager to free memory
-    // TODO: we probably don't even have to store the request level hook manager in the first place
-    if (hookManagersMap && allHookManagers.length > 0) {
-      const requestHookManagerKey = item.pathname;
-      hookManagersMap.delete(requestHookManagerKey);
-    }
+    // Request-level afterResponse hooks
+    await executeHooksForLevel(requestHooks, HOOK_EVENTS.HTTP_AFTER_RESPONSE, afterResponseEventData);

    // run post-response vars
    const postResponseVars = get(item, 'request.vars.res');
@@ -853,11 +846,6 @@ const runSingleRequest = async function (
      shouldStopRunnerExecution
    };
  } catch (err) {
-    // Clean up request-level hook manager on error
-    if (hookManagersMap) {
-      const requestHookManagerKey = item.pathname;
-      hookManagersMap.delete(requestHookManagerKey);
-    }
    console.log(chalk.red(stripExtension(relativeItemPathname)) + chalk.dim(` (${err.message})`));
    return {
      test: {