Skip to content

Commit

Permalink
Fixes for review
Browse files Browse the repository at this point in the history
  • Loading branch information
@pkukielka
pkukielka committed Feb 14, 2025
1 parent 377ee05 commit 84f8d4e
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 69 deletions.
104 changes: 45 additions & 59 deletions lib/shared/src/cody-ignore/context-filters-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
type ContextFilters,
EXCLUDE_EVERYTHING_CONTEXT_FILTERS,
INCLUDE_EVERYTHING_CONTEXT_FILTERS,
type RefetchIntervalHint,
TRANSIENT_REFETCH_INTERVAL_HINT,
} from '../sourcegraph-api/graphql/client'
import { wrapInActiveSpan } from '../tracing'
import { createSubscriber } from '../utils'
Expand Down Expand Up @@ -68,20 +70,6 @@ type IsRepoNameIgnored = boolean
// the remote applies Cody Context Filters rules.
const allowedSchemes = new Set(['http', 'https'])

type ResultLifetime = 'ephemeral' | 'durable'

// hasAllowEverythingFilters, hasIgnoreEverythingFilters relies on === equality
// for fast paths.
function canonicalizeContextFilters(filters: ContextFilters | Error): ContextFilters | Error {
if (isEqual(filters, INCLUDE_EVERYTHING_CONTEXT_FILTERS)) {
return INCLUDE_EVERYTHING_CONTEXT_FILTERS
}
if (isEqual(filters, EXCLUDE_EVERYTHING_CONTEXT_FILTERS)) {
return EXCLUDE_EVERYTHING_CONTEXT_FILTERS
}
return filters
}

export class ContextFiltersProvider implements vscode.Disposable {
static repoNameResolver: {
getRepoNamesContainingUri: GetRepoNamesContainingUri
Expand All @@ -97,8 +85,10 @@ export class ContextFiltersProvider implements vscode.Disposable {
private cache = new LRUCache<RepoName, IsRepoNameIgnored>({ max: 128 })

private lastFetchDelay = 0
private lastResultLifetime: ResultLifetime | undefined = undefined
private fetchIntervalId: NodeJS.Timeout | undefined | number
private lastFetchTimestamp = Date.now()
private lastResultLifetime: Promise<RefetchIntervalHint> = Promise.resolve(
TRANSIENT_REFETCH_INTERVAL_HINT
)

// Visible for testing.
public get timerStateForTest() {
Expand All @@ -108,20 +98,17 @@ export class ContextFiltersProvider implements vscode.Disposable {
private readonly contextFiltersSubscriber = createSubscriber<ContextFilters | Error>()
public readonly onContextFiltersChanged = this.contextFiltersSubscriber.subscribe

// Fetches context filters and updates the cached filter results. Returns
// 'ephemeral' if the results should be re-queried sooner because they
// are transient results arising from, say, a network error; or 'durable'
// if the results can be cached for a while.
private async fetchContextFilters(): Promise<ResultLifetime> {
// Fetches context filters and updates the cached filter results
private async fetchContextFilters(): Promise<RefetchIntervalHint> {
try {
const { filters, transient } = await graphqlClient.contextFilters()
const { filters, refetchIntervalHint } = await graphqlClient.contextFilters()
this.setContextFilters(filters)
return transient ? 'ephemeral' : 'durable'
return refetchIntervalHint
} catch (error) {
logError('ContextFiltersProvider', 'fetchContextFilters', {
verbose: error,
})
return 'ephemeral'
return TRANSIENT_REFETCH_INTERVAL_HINT
}
}

Expand All @@ -142,7 +129,7 @@ export class ContextFiltersProvider implements vscode.Disposable {

this.cache.clear()
this.parsedContextFilters = null
this.lastContextFiltersResponse = canonicalizeContextFilters(contextFilters)
this.lastContextFiltersResponse = contextFilters

// Disable logging for unit tests. Retain for manual debugging of enterprise issues.
if (!cenv.CODY_TESTING_LOG_SUPRESS_VERBOSE) {
Expand All @@ -151,47 +138,50 @@ export class ContextFiltersProvider implements vscode.Disposable {
})
}

const filters = isError(contextFilters) ? EXCLUDE_EVERYTHING_CONTEXT_FILTERS : contextFilters
this.parsedContextFilters = {
include: filters.include?.map(parseContextFilterItem) || null,
exclude: filters.exclude?.map(parseContextFilterItem) || null,
}
this.parsedContextFilters = this.parseContextFilter(
isError(contextFilters) ? EXCLUDE_EVERYTHING_CONTEXT_FILTERS : contextFilters
)

this.contextFiltersSubscriber.notify(this.lastContextFiltersResponse)
}

private isTesting = false
private parseContextFilter(contextFilters: ContextFilters): ParsedContextFilters {
return {
include: contextFilters.include?.map(parseContextFilterItem) || null,
exclude: contextFilters.exclude?.map(parseContextFilterItem) || null,
}
}

/**
* Overrides context filters for testing.
*/
public setTestingContextFilters(contextFilters: ContextFilters | null): void {
if (contextFilters === null) {
this.isTesting = false
this.reset() // reset context filters to the value from the Sourcegraph API
} else {
this.isTesting = true
this.setContextFilters(contextFilters)
}
}

private startRefetchTimer(intervalHint: ResultLifetime): void {
if (this.lastResultLifetime === intervalHint) {
this.lastFetchDelay *= REFETCH_INTERVAL_MAP[intervalHint].backoff
} else {
this.lastFetchDelay = REFETCH_INTERVAL_MAP[intervalHint].initialInterval
this.lastResultLifetime = intervalHint
}
this.fetchIntervalId = setTimeout(async () => {
this.startRefetchTimer(await this.fetchContextFilters())
}, this.lastFetchDelay)
}
private async fetchIfNeeded(): Promise<void> {
this.lastResultLifetime = this.lastResultLifetime.then(async intervalHint => {
if (this.lastFetchTimestamp + this.lastFetchDelay < Date.now()) {
this.lastFetchTimestamp = Date.now()
const nextIntervalHint = await this.fetchContextFilters()

private async fetchIfNeeded(forceFetch = false): Promise<void> {
if (forceFetch || (!this.fetchIntervalId && !this.isTesting)) {
const intervalHint = await this.fetchContextFilters()
this.startRefetchTimer(intervalHint)
}
if (isEqual(intervalHint, nextIntervalHint)) {
this.lastFetchDelay *= nextIntervalHint.backoff
} else {
this.lastFetchDelay = nextIntervalHint.initialInterval
console.log(this.lastFetchDelay)
}

return nextIntervalHint
}
return intervalHint
})

await this.lastResultLifetime
}

public async isRepoNameIgnored(repoName: string): Promise<boolean> {
Expand Down Expand Up @@ -230,11 +220,11 @@ export class ContextFiltersProvider implements vscode.Disposable {
return isIgnored
}

public async isUriIgnored(uri: vscode.Uri, forceFetch = false): Promise<IsIgnored> {
public async isUriIgnored(uri: vscode.Uri): Promise<IsIgnored> {
if (isDotCom(currentAuthStatus())) {
return false
}
await this.fetchIfNeeded(forceFetch)
await this.fetchIfNeeded()

if (allowedSchemes.has(uri.scheme) || this.hasAllowEverythingFilters()) {
return false
Expand Down Expand Up @@ -278,17 +268,13 @@ export class ContextFiltersProvider implements vscode.Disposable {
}

private reset(): void {
this.lastFetchTimestamp = Date.now()
this.lastResultLifetime = Promise.resolve(TRANSIENT_REFETCH_INTERVAL_HINT)
this.lastFetchDelay = 0
this.lastResultLifetime = undefined
this.lastContextFiltersResponse = null
this.parsedContextFilters = null

this.cache.clear()

if (this.fetchIntervalId) {
clearTimeout(this.fetchIntervalId)
this.fetchIntervalId = undefined
}
}

public dispose(): void {
Expand All @@ -298,12 +284,12 @@ export class ContextFiltersProvider implements vscode.Disposable {
private hasAllowEverythingFilters(): boolean {
return (
isDotCom(currentAuthStatus()) ||
this.lastContextFiltersResponse === INCLUDE_EVERYTHING_CONTEXT_FILTERS
isEqual(this.lastContextFiltersResponse, INCLUDE_EVERYTHING_CONTEXT_FILTERS)
)
}

private hasIgnoreEverythingFilters() {
return this.lastContextFiltersResponse === EXCLUDE_EVERYTHING_CONTEXT_FILTERS
return isEqual(this.lastContextFiltersResponse, EXCLUDE_EVERYTHING_CONTEXT_FILTERS)
}

public toDebugObject() {
Expand Down
41 changes: 33 additions & 8 deletions lib/shared/src/sourcegraph-api/graphql/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,21 @@ export type GraphQLAPIClientConfig = PickResolvedConfiguration<{
clientState: 'anonymousUserID'
}>

export interface RefetchIntervalHint {
initialInterval: number
backoff: number
}

export const DURABLE_REFETCH_INTERVAL_HINT: RefetchIntervalHint = {
initialInterval: 60 * 60 * 1000, // 1 hour
backoff: 1.0,
}

export const TRANSIENT_REFETCH_INTERVAL_HINT: RefetchIntervalHint = {
initialInterval: 7 * 1000, // 7 seconds
backoff: 1.5,
}

const QUERY_TO_NAME_REGEXP = /^\s*(?:query|mutation)\s+(\w+)/m

export class SourcegraphGraphQLAPIClient {
Expand Down Expand Up @@ -1229,7 +1244,7 @@ export class SourcegraphGraphQLAPIClient {

public async contextFilters(): Promise<{
filters: ContextFilters | Error
transient: boolean
refetchIntervalHint: RefetchIntervalHint
}> {
// CONTEXT FILTERS are only available on Sourcegraph 5.3.3 and later.
const minimumVersion = '5.3.3'
Expand All @@ -1245,15 +1260,15 @@ export class SourcegraphGraphQLAPIClient {
// Exclude everything in case of an unexpected error.
return {
filters: error,
transient: true,
refetchIntervalHint: TRANSIENT_REFETCH_INTERVAL_HINT,
}
}
const insiderBuild = version.length > 12 || version.includes('dev')
const isValidVersion = insiderBuild || semver.gte(version, minimumVersion)
if (!isValidVersion) {
return {
filters: INCLUDE_EVERYTHING_CONTEXT_FILTERS,
transient: false,
refetchIntervalHint: DURABLE_REFETCH_INTERVAL_HINT,
}
}

Expand All @@ -1266,21 +1281,21 @@ export class SourcegraphGraphQLAPIClient {
if (data?.site?.codyContextFilters?.raw === null) {
return {
filters: INCLUDE_EVERYTHING_CONTEXT_FILTERS,
transient: false,
refetchIntervalHint: DURABLE_REFETCH_INTERVAL_HINT,
}
}

if (data?.site?.codyContextFilters?.raw) {
return {
filters: data.site.codyContextFilters.raw,
transient: false,
refetchIntervalHint: DURABLE_REFETCH_INTERVAL_HINT,
}
}

// Exclude everything in case of an unexpected response structure.
return {
filters: EXCLUDE_EVERYTHING_CONTEXT_FILTERS,
transient: true,
refetchIntervalHint: TRANSIENT_REFETCH_INTERVAL_HINT,
}
})

Expand All @@ -1290,15 +1305,25 @@ export class SourcegraphGraphQLAPIClient {
if (hasOutdatedAPIErrorMessages(error)) {
return {
filters: INCLUDE_EVERYTHING_CONTEXT_FILTERS,
transient: false,
refetchIntervalHint: DURABLE_REFETCH_INTERVAL_HINT,
}
}

if (isNeedsAuthChallengeError(error)) {
return {
filters: error,
refetchIntervalHint: {
initialInterval: 3 * 1000, // 3 seconds
backoff: 1,
},
}
}

logError('SourcegraphGraphQLAPIClient', 'contextFilters', error.message)
// Exclude everything in case of an unexpected error.
return {
filters: error,
transient: true,
refetchIntervalHint: TRANSIENT_REFETCH_INTERVAL_HINT,
}
}

Expand Down
6 changes: 4 additions & 2 deletions vscode/src/auth/auth.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('showSignInMenu', () => {

await showSignInMenu()
expect(mockShowInputBox).not.toHaveBeenCalled()
expect(mockShowErrorMessage).toHaveBeenCalledWith('Sourcegraph is unreachable')
expect(mockShowErrorMessage).toHaveBeenCalledWith('Network Error: Sourcegraph is unreachable')
})

it('shows access token input box when authentication fails with invalid access token error', async () => {
Expand Down Expand Up @@ -89,7 +89,9 @@ describe('showSignInMenu', () => {

await showSignInMenu()
expect(mockShowInputBox).toHaveBeenCalled()
expect(mockShowErrorMessage).toHaveBeenCalledWith('The access token is invalid or has expired')
expect(mockShowErrorMessage).toHaveBeenCalledWith(
'Invalid Access Token: The access token is invalid or has expired'
)
})
})

Expand Down

0 comments on commit 84f8d4e

Please sign in to comment.