Skip to content

Conversation

@cte
Copy link
Collaborator

@cte cte commented Jun 13, 2025

Summary

This PR fixes issue #4664 where the apply_diff tool returns an unhelpful 'undefined' error message when validation fails with non-Error objects or Error objects without a message property.

Changes Made

Core Fix

  • Fixed error handling in presentAssistantMessage.ts: Updated the error handling logic in the tool validation catch block to properly handle cases where error.message is undefined
  • Added fallback error serialization: When error.message is undefined, the code now uses JSON.stringify(serializeError(error)) as a fallback to provide meaningful error information

Test Coverage

  • Added comprehensive test suite: Created presentAssistantMessage.test.ts with tests covering:
    • Error objects with message property (normal case)
    • Non-Error objects without message property
    • Error objects without message property
    • String errors
    • Null/undefined errors
    • Normal operation validation

Technical Details

The bug occurred in src/core/assistant-message/presentAssistantMessage.ts at lines 355-359 where the code was directly accessing error.message without checking if it exists:

Testing

  • ✅ All existing tests pass
  • ✅ New comprehensive test suite covers all error scenarios
  • ✅ Manual testing confirms the fix works as expected
  • ✅ TypeScript compilation successful
  • ✅ Linting passes

Impact

This fix ensures that users will always receive meaningful error messages instead of seeing 'undefined' when tool validation fails, improving the debugging experience and user experience overall.

Fixes #4664


Important

Fixes issue #4664 by improving error handling in presentAssistantMessage.ts and adding comprehensive tests.

  • Core Fix:
    • Update error handling in presentAssistantMessage.ts to handle undefined error.message by using JSON.stringify(serializeError(error)).
  • Test Coverage:
    • Add presentAssistantMessage.test.ts to test error handling for various error types, including non-Error objects and undefined errors.
  • Misc:
    • Refine directory exclusion logic in list-files.ts by specifying directories to ignore in non-recursive searches.

This description was created by Ellipsis for 714ef08. You can customize this summary. It will automatically update as commits are pushed.

cte added 2 commits June 13, 2025 10:56
Fixes #4647

The issue was that the listFiles function was using a blanket '.*' pattern
to exclude all hidden directories (directories starting with a dot) at the
ripgrep level, before the RooIgnoreController could process them.

This meant that .next folders were being filtered out during file discovery,
preventing the .rooignore patterns from working correctly for these directories.

Changes:
- Replaced the generic '.*' pattern in DIRS_TO_IGNORE with specific patterns
  for common hidden directories (.git, .svn, .hg, .bzr, .vscode, .idea)
- Removed special handling for the '.*' pattern in directory filtering logic
- Added test case to verify .next folders can now be properly controlled via .rooignore

This allows users to specify patterns like 'example-nextjs/.next/' in their
.rooignore file and have them work as expected, while still filtering out
common hidden directories that should typically be ignored.
- Fixed error handling in presentAssistantMessage.ts to properly handle non-Error objects and Error objects without message property
- Added comprehensive test coverage for different error scenarios
- Now uses serializeError as fallback when error.message is undefined
- Prevents unhelpful 'undefined' error messages from being displayed to users
@cte cte requested review from jr and mrubens as code owners June 13, 2025 19:22
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 13, 2025
@cte cte added the roomote label Jun 13, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 13, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 13, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 13, 2025
@daniel-lxs
Copy link
Member

Solved by #4674

@daniel-lxs daniel-lxs closed this Jun 13, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 13, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 13, 2025
@cte cte deleted the fix-apply-diff-undefined-error branch June 16, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Needs Preliminary Review roomote size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Bug: apply_diff tool returns 'undefined' error message on failure and hangs

4 participants