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.
This commit is contained in:
Abhishek S Lal
2026-03-13 23:47:09 +05:30
committed by GitHub
parent 994d51b680
commit 1e25825e74
3 changed files with 148 additions and 43 deletions

View File

@@ -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) => {

View File

@@ -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);
});
});

View File

@@ -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,