Skip to content

feat: add timeout in searching text in files #1233

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+76 −13
Open

feat: add timeout in searching text in files #1233

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f7b7f47
feat: add timeout handling for file and text search operations
bryanchen-d Oct 3, 2025
b176a12
feat: add timeout handling for search and rendering operations in Fin…
bryanchen-d Oct 3, 2025
e1d6814
use raceTimeout and raceCancellation
bryanchen-d Oct 6, 2025
a4b3dde
Updated per comments
bryanchen-d Oct 6, 2025
86bdb0e
extract a raceTimeoutAndCancellationError()
bryanchen-d Oct 6, 2025
7f0cd35
enhance timeout handling and cancellation for file and text search op…
bryanchen-d Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions src/extension/tools/node/findFilesTool.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { URI } from '../../../util/vs/base/common/uri';
import * as l10n from '@vscode/l10n';
import { ISearchService } from '../../../platform/search/common/searchService';
import { IWorkspaceService } from '../../../platform/workspace/common/workspaceService';
import { raceCancellation, raceTimeout } from '../../../util/vs/base/common/async';
import { CancellationToken } from '../../../util/vs/base/common/cancellation';
import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation';
import { ExtendedLanguageModelToolResult, LanguageModelPromptTsxPart, MarkdownString } from '../../../vscodeTypes';
Expand Down Expand Up @@ -40,14 +41,38 @@ export class FindFilesTool implements ICopilotTool<IFindFilesToolParams> {
// The input _should_ be a pattern matching inside a workspace, folder, but sometimes we get absolute paths, so try to resolve them
const pattern = inputGlobToPattern(options.input.query, this.workspaceService);

const results = await this.searchService.findFiles(pattern, undefined, token);
// try find files with a timeout of 10s
// TODO: consider making the timeout configurable
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

token
),
timeoutInMs
);

if (results === undefined) {
throw new Error('Timeout in searching files');
}

checkCancellation(token);

const maxResults = options.input.maxResults ?? 20;
const resultsToShow = results.slice(0, maxResults);
const result = new ExtendedLanguageModelToolResult([
new LanguageModelPromptTsxPart(
await renderPromptElementJSON(this.instantiationService, FindFilesResult, { fileResults: resultsToShow, totalResults: results.length }, options.tokenizationOptions, token))]);
// Render the prompt element with a timeout
const prompt = await raceTimeout(
raceCancellation(
renderPromptElementJSON(this.instantiationService, FindFilesResult, { fileResults: resultsToShow, totalResults: results.length }, options.tokenizationOptions, token),
token),
timeoutInMs
);

if (prompt === undefined) {
throw new Error('Timeout in rendering prompt');
}

const result = new ExtendedLanguageModelToolResult([new LanguageModelPromptTsxPart(prompt)]);
const query = `\`${options.input.query}\``;
result.toolResultMessage = resultsToShow.length === 0 ?
new MarkdownString(l10n.t`Searched for files matching ${query}, no matches`) :
Expand Down
45 changes: 39 additions & 6 deletions src/extension/tools/node/findTextInFilesTool.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { IPromptPathRepresentationService } from '../../../platform/prompts/comm
import { ISearchService } from '../../../platform/search/common/searchService';
import { IWorkspaceService } from '../../../platform/workspace/common/workspaceService';
import { asArray } from '../../../util/vs/base/common/arrays';
import { raceCancellation, raceTimeout } from '../../../util/vs/base/common/async';
import { CancellationToken } from '../../../util/vs/base/common/cancellation';
import { count } from '../../../util/vs/base/common/strings';
import { URI } from '../../../util/vs/base/common/uri';
Expand Down Expand Up @@ -51,14 +52,45 @@ export class FindTextInFilesTool implements ICopilotTool<IFindTextInFilesToolPar
const maxResults = Math.min(options.input.maxResults ?? 20, MaxResultsCap);
const isRegExp = options.input.isRegexp ?? true;
const queryIsValidRegex = this.isValidRegex(options.input.query);
let results = await this.searchAndCollectResults(options.input.query, isRegExp, patterns, maxResults, token);
if (!results.length && queryIsValidRegex) {
results = await this.searchAndCollectResults(options.input.query, !isRegExp, patterns, maxResults, token);

// try find text with a timeout of 10s
// TODO: consider making the timeout configurable
const timeoutInMs = 10_000;
let results = await raceTimeout(
raceCancellation(
this.searchAndCollectResults(options.input.query, isRegExp, patterns, maxResults, token),
token
),
timeoutInMs
);

if (results && !results.length && queryIsValidRegex) {
results = await raceTimeout(
raceCancellation(
this.searchAndCollectResults(options.input.query, !isRegExp, patterns, maxResults, token),
token
),
timeoutInMs
);
}

if (results === undefined) {
throw new Error('Timeout in searching text in files');
}

const prompt = await raceTimeout(
raceCancellation(
renderPromptElementJSON(this.instantiationService, FindTextInFilesResult, { textResults: results, maxResults, askedForTooManyResults: Boolean(askedForTooManyResults) }, options.tokenizationOptions, token),
token
),
timeoutInMs
);

if (prompt === undefined) {
throw new Error('Timeout in rendering prompt');
}

const result = new ExtendedLanguageModelToolResult([
new LanguageModelPromptTsxPart(
await renderPromptElementJSON(this.instantiationService, FindTextInFilesResult, { textResults: results, maxResults, askedForTooManyResults: Boolean(askedForTooManyResults) }, options.tokenizationOptions, token))]);
const result = new ExtendedLanguageModelToolResult([new LanguageModelPromptTsxPart(prompt)]);
const textMatches = results.flatMap(r => {
if ('ranges' in r) {
return asArray(r.ranges).map(rangeInfo => new Location(r.uri, rangeInfo.sourceRange));
Expand Down Expand Up @@ -99,6 +131,7 @@ export class FindTextInFilesTool implements ICopilotTool<IFindTextInFilesToolPar
token);
const results: vscode.TextSearchResult2[] = [];
for await (const item of searchResult.results) {
await new Promise(r => setTimeout(r, 5000)); // yield to allow cancellation to be processed
Copy link
Member

Choose a reason for hiding this comment

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

What's this? We need to wait 5s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, this shouldn't be checked in, I was using it to validate the timeout case

checkCancellation(token);
results.push(item);
}
Expand Down
Loading