Skip to content

Conversation

bryanchen-d
Copy link
Contributor

@bryanchen-d bryanchen-d commented Oct 3, 2025

Add timeout in findFiles and findTextInFiles tool so they won't hang the chat.

Fixes: microsoft/vscode#262743

@bryanchen-d bryanchen-d requested a review from roblourens October 3, 2025 23:30
@bryanchen-d bryanchen-d self-assigned this Oct 6, 2025
@bryanchen-d bryanchen-d force-pushed the brchen/timeout-in-searching branch from 9dec97e to e1d6814 Compare October 6, 2025 18:10
@bryanchen-d bryanchen-d marked this pull request as ready for review October 6, 2025 18:16
@Copilot Copilot AI review requested due to automatic review settings October 6, 2025 18:16
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds timeout functionality to the findFiles and findTextInFiles tools to prevent them from hanging chat operations. The changes implement a 10-second timeout for both search operations and prompt rendering to improve user experience.

Key changes:

  • Added timeout wrapper using raceTimeout and raceCancellation utilities
  • Applied timeout to both file search operations and prompt rendering phases
  • Added consistent error handling when timeouts occur

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/extension/tools/node/findTextInFilesTool.tsx Added timeout logic to text search operations and prompt rendering with proper error handling
src/extension/tools/node/findFilesTool.tsx Added timeout logic to file search operations and prompt rendering with consistent error messaging

const timeoutInMs = 10_000;
const results = await raceTimeout(
raceCancellation(
Promise.resolve(this.searchService.findFiles(pattern, undefined, token)),
Copy link
Preview

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Promise.resolve() wrapper is unnecessary here. The findFiles method likely already returns a Promise, so this adds unnecessary complexity.

Suggested change
Promise.resolve(this.searchService.findFiles(pattern, undefined, token)),
this.searchService.findFiles(pattern, undefined, token),

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the findFiles returns a Thenable<> which doesn't fit the required Promise<> type, and fails compilation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense for the search service to return a Promise instead (this comes from our extension API but our searchService could do the Promise.resolve. You can do that if it makes things cleaner, you don't have to

@vs-code-engineering vs-code-engineering bot added this to the October 2025 milestone Oct 6, 2025
@roblourens
Copy link
Member

Please link the issue from the PR so we can keep track of what change goes with which issue. If you say fixes microsoft/vscode#123 then the issue will be automatically closed when the PR is merged. microsoft/vscode#262743

@bryanchen-d bryanchen-d added this pull request to the merge queue Oct 7, 2025
Merged via the queue into main with commit 1084547 Oct 7, 2025
16 checks passed
@bryanchen-d bryanchen-d deleted the brchen/timeout-in-searching branch October 7, 2025 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agent freezes on "Searching text for ...
2 participants