From 1e25825e74cb3cc46a63d25897f8972a1aec0860 Mon Sep 17 00:00:00 2001 From: Abhishek S Lal Date: Fri, 13 Mar 2026 23:47:09 +0530 Subject: [PATCH] fix(collection-watcher): prevent crash when deleting collections (#7470) * fix(collection-watcher): guard against events firing after collection deletion When deleting an OpenAPI-synced collection, saveBrunoConfig() writes to bruno.json which creates buffered chokidar events (80ms stabilityThreshold). If the collection directory is removed before those events fire, getCollectionFormat() throws "No collection configuration found" for each .bru file in the collection. Add fs.existsSync(collectionPath) guards in the change, unlink, and unlinkDir handlers to bail out early when the collection root no longer exists. * fix(workspaces): ensure collection watcher stops before deletion Added logic to remove the collection from the watcher when deleting files, preventing chokidar from firing events on a directory that is being removed. This change enhances stability during collection deletions by ensuring the watcher is properly managed. * refactor(workspaces): remove redundant collection watcher logic during deletion Eliminated the logic for stopping the collection watcher before deletion, streamlining the action for removing collections from workspaces. This change simplifies the code and maintains functionality without unnecessary complexity. * test(collection): add integration test for collection deletion functionality Introduced a new test suite to verify the deletion of collections from the workspace overview. The test ensures that collections are properly removed from both the UI and the file system, confirming the absence of any uncaught errors during the deletion process. This addition enhances the test coverage for collection management features. * feat(collection): implement deleteCollectionFromOverview utility function Added a new utility function to delete a collection directly from the workspace overview page. This function encapsulates the steps required to navigate the UI, confirm deletion, and ensure the collection is removed from both the interface and the file system. Updated the corresponding test to utilize this new function, enhancing code reusability and test clarity. * fix(collection-watcher): add guards for collection path existence and error handling Enhanced the unlink and unlinkDir functions to check for the existence of the collection path before proceeding. Added error handling for the getCollectionFormat function to prevent crashes when the collection format cannot be retrieved. These changes improve stability and robustness during collection deletion operations. --- .../src/app/collection-watcher.js | 112 +++++++++++------- .../delete/delete-collection.spec.ts | 42 +++++++ tests/utils/page/actions.ts | 37 ++++++ 3 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 tests/collection/delete/delete-collection.spec.ts diff --git a/packages/bruno-electron/src/app/collection-watcher.js b/packages/bruno-electron/src/app/collection-watcher.js index a1f5621c2..c4aece50a 100644 --- a/packages/bruno-electron/src/app/collection-watcher.js +++ b/packages/bruno-electron/src/app/collection-watcher.js @@ -552,58 +552,84 @@ const change = async (win, pathname, collectionUid, collectionPath) => { }; const unlink = (win, pathname, collectionUid, collectionPath) => { - console.log(`watcher unlink: ${pathname}`); - - if (isEnvironmentsFolder(pathname, collectionPath)) { - return unlinkEnvironmentFile(win, pathname, collectionUid); - } - - const format = getCollectionFormat(collectionPath); - if (hasRequestExtension(pathname, format)) { - const basename = path.basename(pathname); - const dirname = path.dirname(pathname); - - if (basename === 'opencollection.yml' && path.normalize(dirname) === path.normalize(collectionPath)) { + try { + if (!fs.existsSync(collectionPath)) { return; } + console.log(`watcher unlink: ${pathname}`); - const file = { - meta: { - collectionUid, - pathname, - name: basename + if (isEnvironmentsFolder(pathname, collectionPath)) { + return unlinkEnvironmentFile(win, pathname, collectionUid); + } + + let format; + try { + format = getCollectionFormat(collectionPath); + } catch (error) { + console.error(`Error getting collection format for: ${collectionPath}`, error); + return; + } + if (hasRequestExtension(pathname, format)) { + const basename = path.basename(pathname); + const dirname = path.dirname(pathname); + + if (basename === 'opencollection.yml' && path.normalize(dirname) === path.normalize(collectionPath)) { + return; } - }; - win.webContents.send('main:collection-tree-updated', 'unlink', file); + + const file = { + meta: { + collectionUid, + pathname, + name: basename + } + }; + win.webContents.send('main:collection-tree-updated', 'unlink', file); + } + } catch (err) { + console.error(`Error processing unlink event for: ${pathname}`, err); } }; const unlinkDir = async (win, pathname, collectionUid, collectionPath) => { - const envDirectory = path.join(collectionPath, 'environments'); - - if (path.normalize(pathname) === path.normalize(envDirectory)) { - return; - } - - const format = getCollectionFormat(collectionPath); - const folderFilePath = path.join(pathname, `folder.${format}`); - - let name = path.basename(pathname); - - if (fs.existsSync(folderFilePath)) { - let folderFileContent = fs.readFileSync(folderFilePath, 'utf8'); - let folderData = await parseFolder(folderFileContent, { format }); - name = folderData?.meta?.name || name; - } - - const directory = { - meta: { - collectionUid, - pathname, - name + try { + if (!fs.existsSync(collectionPath)) { + return; } - }; - win.webContents.send('main:collection-tree-updated', 'unlinkDir', directory); + const envDirectory = path.join(collectionPath, 'environments'); + + if (path.normalize(pathname) === path.normalize(envDirectory)) { + return; + } + + let format; + try { + format = getCollectionFormat(collectionPath); + } catch (error) { + console.error(`Error getting collection format for: ${collectionPath}`, error); + return; + } + const folderFilePath = path.join(pathname, `folder.${format}`); + + let name = path.basename(pathname); + + if (fs.existsSync(folderFilePath)) { + let folderFileContent = fs.readFileSync(folderFilePath, 'utf8'); + let folderData = await parseFolder(folderFileContent, { format }); + name = folderData?.meta?.name || name; + } + + const directory = { + meta: { + collectionUid, + pathname, + name + } + }; + win.webContents.send('main:collection-tree-updated', 'unlinkDir', directory); + } catch (err) { + console.error(`Error processing unlinkDir event for: ${pathname}`, err); + } }; const onWatcherSetupComplete = (win, watchPath, collectionUid, watcher) => { diff --git a/tests/collection/delete/delete-collection.spec.ts b/tests/collection/delete/delete-collection.spec.ts new file mode 100644 index 000000000..bb033672d --- /dev/null +++ b/tests/collection/delete/delete-collection.spec.ts @@ -0,0 +1,42 @@ +import fs from 'fs'; +import path from 'path'; +import { test, expect } from '../../../playwright'; +import { createCollection, createRequest, deleteCollectionFromOverview } from '../../utils/page'; + +test.describe('Delete collection', () => { + test('Delete collection from workspace overview removes files from disk', async ({ page, createTmpDir }) => { + const collectionName = 'delete-test-collection'; + const tmpDir = await createTmpDir(collectionName); + const collectionPath = path.join(tmpDir, collectionName); + + // Create a collection with a request + await createCollection(page, collectionName, tmpDir); + await createRequest(page, 'ping', collectionName, { url: 'http://localhost:8081/ping' }); + + // Verify collection directory exists on disk + expect(fs.existsSync(collectionPath)).toBe(true); + + // Capture any uncaught errors during deletion + const pageErrors: Error[] = []; + page.on('pageerror', (error) => pageErrors.push(error)); + + // Navigate to Workspace and delete collection from overview + await deleteCollectionFromOverview(page, collectionName); + + // Verify collection is removed from overview + await expect( + page.locator('.collection-card').filter({ hasText: collectionName }) + ).not.toBeVisible(); + + // Verify collection is removed from sidebar + await expect( + page.locator('#sidebar-collection-name').filter({ hasText: collectionName }) + ).not.toBeVisible(); + + // Verify collection directory is deleted from disk + expect(fs.existsSync(collectionPath)).toBe(false); + + // Verify no uncaught JS errors occurred during deletion + expect(pageErrors).toHaveLength(0); + }); +}); diff --git a/tests/utils/page/actions.ts b/tests/utils/page/actions.ts index 30fd5ca4c..c19d9b2e8 100644 --- a/tests/utils/page/actions.ts +++ b/tests/utils/page/actions.ts @@ -323,6 +323,42 @@ const deleteRequest = async (page, requestName: string, collectionName: string) }); }; +/** + * Delete a collection permanently from disk via the workspace overview page + * @param page - The page object + * @param collectionName - The name of the collection to delete + * @returns void + */ +const deleteCollectionFromOverview = async (page: Page, collectionName: string) => { + await test.step(`Delete collection "${collectionName}" from workspace overview`, async () => { + // Navigate to workspace overview + await page.locator('.home-button').click(); + const overviewTab = page.locator('.request-tab').filter({ hasText: 'Overview' }); + await overviewTab.click(); + + // Find the collection card and open its menu + const collectionCard = page.locator('.collection-card').filter({ hasText: collectionName }); + await collectionCard.waitFor({ state: 'visible', timeout: 5000 }); + await collectionCard.locator('.collection-menu').click(); + + // Click Delete from the dropdown + await page.locator('.dropdown-item').filter({ hasText: 'Delete' }).click(); + + // Wait for delete confirmation modal + const deleteModal = page.locator('.bruno-modal').filter({ hasText: 'Delete Collection' }); + await deleteModal.waitFor({ state: 'visible', timeout: 5000 }); + + // Type 'delete' to confirm + await deleteModal.locator('#delete-confirm-input').fill('delete'); + + // Click the Delete button + await deleteModal.getByRole('button', { name: 'Delete', exact: true }).click(); + + // Wait for modal to close + await deleteModal.waitFor({ state: 'hidden', timeout: 10000 }); + }); +}; + /** * Import a collection from a file * @param page - The page object @@ -1020,6 +1056,7 @@ export { createTransientRequest, fillRequestUrl, deleteRequest, + deleteCollectionFromOverview, importCollection, removeCollection, createFolder,