Skip to content

Commit

Permalink
Chat: Fix hanging issue in repository name resolution for workspaces (#…
Browse files Browse the repository at this point in the history
…5808)

CLOSE:
https://linear.app/sourcegraph/issue/CODY-3971/investigate-submit-button-issue-in-chat-for-enterprise-users

The `remoteReposForAllWorkspaceFolders` function was incorrectly
handling non-array results, such as errors or pending operations, which
could cause the function to hang indefinitely. This change filters out
these non-array results and returns an empty array instead, ensuring the
function behaves correctly.

Description:

This PR addresses an issue where Cody would hang when encountering a
workspace folder without a remote origin URL. The problem was caused by
incorrect error handling in the repository name resolution process,
which led to unresolved promises and pending operations that cause the
function to hang indefinitely.

Root Cause:

- The RepoNameResolver class was catching errors in
getUniqueRemoteUrlsCached and getRepoNameCached methods but returning
empty arrays or null values instead of properly propagating the errors.
- These non-error, non-array results were being interpreted as pending
operations in the remoteReposForAllWorkspaceFolders observable, causing
the process to hang.

Solution:

1. Modified getUniqueRemoteUrlsCached to return null instead of an empty
array on error.
2. Updated getRepoNameCached to handle null cases explicitly and ensure
consistent return values.
3. Adjusted getRepoNamesContainingUri to filter out null values before
returning results.
4. Removed the premature pendingOperation check in the
abortableOperation within remoteReposForAllWorkspaceFolders.

These changes ensure that errors and missing data are handled
consistently throughout the resolver process, preventing the hanging
behavior and allowing the extension to continue functioning even when
some workspace folders lack remote URLs or other errors that block the
extension from working.

## Test plan

<!-- Required. See
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles.
-->

Manual Testing:

1. Verified that the extension no longer hangs when processing
workspaces without remote URLs.
2. Confirmed that repository name resolution works correctly for valid
remote URLs.
3. Tested with mixed scenarios of workspaces with and without remote
URLs.

Before: if you open a repository that’s working with Cody on S2, and
then run git remote remove origin to remove the origin from the repo
locally, chat will stop working after reloading the Window.

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->

Chat: Fix an issue in repository name resolution for workspaces that
caused Chat to hang.
  • Loading branch information
abeatrix authored and hitesh-1997 committed Oct 4, 2024
1 parent 4655745 commit 57a9c1f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
2 changes: 2 additions & 0 deletions vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ This is a log of all notable changes to Cody for VS Code. [Unreleased] changes a

### Fixed

Chat: Fix an issue in repository name resolution for workspaces that caused Chat to hang. [pull/5808](https://github.com/sourcegraph/cody/pull/5808)

### Changed

## 1.36.2
Expand Down
23 changes: 14 additions & 9 deletions vscode/src/repository/remoteRepos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
fromVSCodeEvent,
graphqlClient,
isError,
pendingOperation,
type pendingOperation,
startWith,
switchMapReplayOperation,
} from '@sourcegraph/cody-shared'
Expand Down Expand Up @@ -61,22 +61,27 @@ export const remoteReposForAllWorkspaceFolders: Observable<
...workspaceFolders.map(folder => repoNameResolver.getRepoNamesContainingUri(folder.uri))
).pipe(
map(repoNamesLists => {
const repoNames = repoNamesLists.flat()
if (repoNames.includes(pendingOperation)) {
return pendingOperation
}
return repoNames as Exclude<(typeof repoNames)[number], typeof pendingOperation>[]
// Filter out non-array results (errors or pendingOperations)
// Flatten the array of arrays and ensure all elements are strings
const completedResults = repoNamesLists
.filter((names): names is string[] => Array.isArray(names))
.flat()
.filter((name): name is string => typeof name === 'string')

// Return completed results if available, otherwise an empty array
// This prevents hanging on pendingOperations for caught errors that returns an empty array/value
// that would be treated as a pendingOperation.
return completedResults.length > 0 ? completedResults : []
}),
abortableOperation(async (repoNames, signal) => {
if (repoNames === pendingOperation) {
return pendingOperation
}
if (repoNames.length === 0) {
// If we pass an empty repoNames array to getRepoIds, we would
// fetch the first 10 repos from the Sourcegraph instance,
// because it would think that argument is not set.
return []
}
// Process the validated results without checking for pendingOperation that
// would cause the abortableOperation to hang indefinitely.
const reposOrError = await graphqlClient.getRepoIds(
repoNames,
MAX_REPO_COUNT,
Expand Down

0 comments on commit 57a9c1f

Please sign in to comment.