- Notifications
You must be signed in to change notification settings - Fork501
feat: exclude errored files (#3746)#3909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
feat: exclude errored files (#3746)#3909
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coderabbitaibot commentedNov 22, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughThis PR introduces functionality to exclude error files from the workspace. A new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/features/apiClient/helpers/modules/sync/local/services/LocalApiClientRecordsSync.ts (1)
595-614:Implementation is solid; consider removing unreachable null check.The method correctly implements the exclude functionality and handles success/error cases appropriately. However, the null check for
adapteron lines 597-602 appears to be unreachable. Looking at thegetAdapter()implementation (line 20-22) and thefsManagerServiceAdapterProvider.get()method (fsManagerServiceAdapter.ts, lines 192-221), the provider either returns a valid adapter instance or throws an error—it never returns null or undefined.If you'd like to simplify, you can remove the null check:
async excludeErrorFile(filePath: string): Promise<{ success: boolean; message?: string }> { const adapter = await this.getAdapter();- if (!adapter) {- return {- success: false,- message: "Failed to get filesystem adapter",- };- } const result = await adapter.excludeFile(filePath); if (result.type === "success") { return { success: true }; } return { success: false, message: result.error.message || "Failed to exclude file", }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/features/apiClient/helpers/modules/sync/interfaces.ts(1 hunks)app/src/features/apiClient/helpers/modules/sync/local/services/LocalApiClientRecordsSync.ts(1 hunks)app/src/features/apiClient/screens/apiClient/components/sidebar/components/ErrorFilesList/ErrorFileslist.tsx(5 hunks)app/src/services/fsManagerServiceAdapter.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-08T19:29:11.895Z
Learnt from: rohanmathur91Repo: requestly/requestly PR: 3658File: app/src/features/apiClient/helpers/modules/sync/localStore/services/LocalStoreRecordsSync.ts:302-333Timestamp: 2025-10-08T19:29:11.895ZLearning: Methods in `LocalStoreRecordsSync` that return `ResponsePromise<T>` use the error format `{ success: false, data: null, error: { type, message } }` rather than the simpler `{ success: false, data: null, message }` format used by older methods. This is the expected and intentional error response structure for `ResponsePromise` return types.Applied to files:
app/src/features/apiClient/helpers/modules/sync/interfaces.tsapp/src/features/apiClient/helpers/modules/sync/local/services/LocalApiClientRecordsSync.ts
📚 Learning: 2025-10-10T08:10:29.153Z
Learnt from: rohanmathur91Repo: requestly/requestly PR: 3658File: app/src/backend/apiClient/batchCreateCollectionRunDetailsInFirebase.ts:29-32Timestamp: 2025-10-10T08:10:29.153ZLearning: In app/src/backend/apiClient/batchCreateCollectionRunDetailsInFirebase.ts, the error path intentionally returns `data: null` rather than an object with `records` and `erroredRecords` arrays. This is a deliberate design decision.Applied to files:
app/src/features/apiClient/helpers/modules/sync/interfaces.tsapp/src/features/apiClient/helpers/modules/sync/local/services/LocalApiClientRecordsSync.ts
📚 Learning: 2025-10-16T10:25:34.657Z
Learnt from: nafees87nRepo: requestly/requestly PR: 3697File: app/src/features/apiClient/store/collectionRunConfig/runConfig.store.ts:157-163Timestamp: 2025-10-16T10:25:34.657ZLearning: In `app/src/features/apiClient/store/collectionRunConfig/runConfig.store.ts`, the `setDataFile` and `removeDataFile` methods should NOT call `setHasUnsavedChanges(true)` as unsaved changes tracking for dataFile operations is handled elsewhere or through a different mechanism.Applied to files:
app/src/features/apiClient/screens/apiClient/components/sidebar/components/ErrorFilesList/ErrorFileslist.tsx
📚 Learning: 2025-10-10T07:10:27.986Z
Learnt from: rohanmathur91Repo: requestly/requestly PR: 3658File: app/src/features/apiClient/helpers/modules/sync/localStore/services/LocalStoreRecordsSync.ts:451-491Timestamp: 2025-10-10T07:10:27.986ZLearning: In `app/src/features/apiClient/helpers/modules/sync/localStore/services/LocalStoreRecordsSync.ts`, the `addRunResult` method does not need to validate that `runResult.runStatus` is "COMPLETED" because this validation is enforced at a higher layer in the application architecture.Applied to files:
app/src/features/apiClient/helpers/modules/sync/local/services/LocalApiClientRecordsSync.ts
🧬 Code graph analysis (2)
app/src/features/apiClient/screens/apiClient/components/sidebar/components/ErrorFilesList/ErrorFileslist.tsx (3)
app/src/features/apiClient/helpers/modules/sync/local/services/types.ts (1)
ErroredRecord(99-104)app/src/features/apiClient/helpers/modules/sync/local/services/LocalApiClientRecordsSync.ts (1)
excludeErrorFile(595-614)app/src/features/apiClient/commands/environments/force-refresh.command.ts (1)
forceRefreshEnvironments(4-23)
app/src/services/fsManagerServiceAdapter.ts (1)
app/src/features/apiClient/helpers/modules/sync/local/services/types.ts (1)
FileSystemResult(15-15)
🔇 Additional comments (3)
app/src/services/fsManagerServiceAdapter.ts (1)
178-182:LGTM! Consistent implementation following established patterns.The
excludeFilemethod correctly:
- Uses the
@FsErrorHandlerdecorator for consistent error handling- Delegates to
invokeProcedureInBGlike all other methods in this adapter- Returns the appropriate
FileSystemResult<void>typeapp/src/features/apiClient/helpers/modules/sync/interfaces.ts (1)
87-87:LGTM! Interface extension follows established conventions.The
excludeErrorFilemethod signature is consistent with similar operations likedeleteRecords(line 41), using the same{ success: boolean; message?: string }return pattern.app/src/features/apiClient/screens/apiClient/components/sidebar/components/ErrorFilesList/ErrorFileslist.tsx (1)
5-5:LGTM! UI integration follows existing patterns and handles all cases appropriately.The exclude functionality is implemented consistently with the existing
handleDeleteErrorFilepattern:
- Calls the repository method
- Conditionally refreshes environments or records based on file type
- Provides clear user feedback via toast (success) or notification (error)
- Dependencies in
useCallbackare correctThe refresh operations (
forceRefreshEnvironments/forceRefreshRecords) are not awaited, which is consistent with the delete handler and creates a fire-and-forget pattern where the toast appears immediately while the refresh happens in the background.Also applies to: 109-110, 119-121, 185-205, 241-241
Uh oh!
There was an error while loading.Please reload this page.
Closes issue:#3746
🎥 Demo Video:
Video/Demo:

Before :
After :
Screen.Recording.2025-11-22.at.7.02.49.PM.mov
📜 Summary of changes:
📝 Overview
Implemented a new feature that allows users to exclude errored files directly from
the UI. When a workspace contains invalid/unsupported files, users can now click an
"Exclude" button to permanently ignore these files in future workspace scans.
✨ What's New
User-Facing Features
🏗️ Architecture
The implementation follows the existing architecture pattern and leverages the
already-present exclusion mechanism:
UI (React) → Repository → Adapter → IPC → Desktop App (FsManager) → requestly.json
How It Works
📁 Files Changed
Desktop App (requestly-desktop-app)
Webapp (requestly/app)
ync.ts
List/ErrorFileslist.tsx
Exclusion Mechanism
Filtering Flow
✅ Testing Checklist
🎨 UI Changes
Before: Errored files only had "Edit" and "Delete" buttons
After: Errored files now have "Edit", "Exclude" (eye-off icon), and "Delete"
buttons
📊 Impact
requestly.json
🔗 Related
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.