Skip to content

Commit

Permalink
Fixes for Language Server sync server (#8514)
Browse files Browse the repository at this point in the history
- Closes #8398

# Important Notes
- The original error caused by a failing `text/openFile` (`openTextFile`) is still present, but (seemingly?) harder to repro now
  • Loading branch information
somebody1234 authored Dec 19, 2023
1 parent 23e0baf commit cbf7248
Show file tree
Hide file tree
Showing 10 changed files with 586 additions and 372 deletions.
54 changes: 54 additions & 0 deletions app/gui2/shared/__tests__/yjsModel.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { rangeEncloses, rangeIntersects, type ContentRange } from 'shared/yjsModel'
import { expect, test } from 'vitest'

type RangeTest = { a: ContentRange; b: ContentRange }

const equalRanges: RangeTest[] = [
{ a: [0, 0], b: [0, 0] },
{ a: [0, 1], b: [0, 1] },
{ a: [-5, 5], b: [-5, 5] },
]

const totalOverlap: RangeTest[] = [
{ a: [0, 1], b: [0, 0] },
{ a: [0, 2], b: [2, 2] },
{ a: [-1, 1], b: [1, 1] },
{ a: [0, 2], b: [0, 1] },
{ a: [-10, 10], b: [-3, 7] },
{ a: [0, 5], b: [1, 2] },
{ a: [3, 5], b: [3, 4] },
]

const reverseTotalOverlap: RangeTest[] = totalOverlap.map(({ a, b }) => ({ a: b, b: a }))

const noOverlap: RangeTest[] = [
{ a: [0, 1], b: [2, 3] },
{ a: [0, 1], b: [-1, -1] },
{ a: [5, 6], b: [2, 3] },
{ a: [0, 2], b: [-2, -1] },
{ a: [-5, -3], b: [9, 10] },
{ a: [-3, 2], b: [3, 4] },
]

const partialOverlap: RangeTest[] = [
{ a: [0, 3], b: [-1, 1] },
{ a: [0, 1], b: [-1, 0] },
{ a: [0, 0], b: [-1, 0] },
{ a: [0, 2], b: [1, 4] },
{ a: [-8, 0], b: [0, 10] },
]

test.each([...equalRanges, ...totalOverlap])('Range $a should enclose $b', ({ a, b }) =>
expect(rangeEncloses(a, b)).toBe(true),
)
test.each([...noOverlap, ...partialOverlap, ...reverseTotalOverlap])(
'Range $a should not enclose $b',
({ a, b }) => expect(rangeEncloses(a, b)).toBe(false),
)
test.each([...equalRanges, ...totalOverlap, ...reverseTotalOverlap, ...partialOverlap])(
'Range $a should intersect $b',
({ a, b }) => expect(rangeIntersects(a, b)).toBe(true),
)
test.each([...noOverlap])('Range $a should not intersect $b', ({ a, b }) =>
expect(rangeIntersects(a, b)).toBe(false),
)
52 changes: 27 additions & 25 deletions app/gui2/shared/languageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ export class LanguageServer extends ObservableV2<Notifications> {
console.dir(params)
}
return await this.client.request({ method, params }, RPC_TIMEOUT_MS)
} catch (e) {
const remoteError = RemoteRpcErrorSchema.safeParse(e)
} catch (error) {
const remoteError = RemoteRpcErrorSchema.safeParse(error)
if (remoteError.success) {
throw new LsRpcError(new RemoteRpcError(remoteError.data), method, params)
} else if (e instanceof Error) {
throw new LsRpcError(e, method, params)
} else if (error instanceof Error) {
throw new LsRpcError(error, method, params)
}
throw e
throw error
} finally {
if (DEBUG_LOG_RPC) {
console.log(`LS [${uuid}] ${method} took ${performance.now() - now}ms`)
Expand Down Expand Up @@ -402,27 +402,29 @@ export class LanguageServer extends ObservableV2<Notifications> {
retry: <T>(cb: () => Promise<T>) => Promise<T> = (f) => f(),
) {
let running = true
;(async () => {
this.on('file/event', callback)
walkFs(this, { rootId, segments }, (type, path) => {
if (
!running ||
type !== 'File' ||
path.segments.length < segments.length ||
segments.some((seg, i) => seg !== path.segments[i])
)
return
callback({
path: { rootId: path.rootId, segments: path.segments.slice(segments.length) },
kind: 'Added',
const self = this
return {
promise: (async () => {
self.on('file/event', callback)
await retry(async () => running && self.acquireReceivesTreeUpdates({ rootId, segments }))
await walkFs(self, { rootId, segments }, (type, path) => {
if (
!running ||
type !== 'File' ||
path.segments.length < segments.length ||
segments.some((segment, i) => segment !== path.segments[i])
)
return
callback({
path: { rootId: path.rootId, segments: path.segments.slice(segments.length) },
kind: 'Added',
})
})
})
await retry(() => this.acquireReceivesTreeUpdates({ rootId, segments }))
if (!running) return
})()
return () => {
running = false
this.off('file/event', callback)
})(),
unsubscribe() {
running = false
self.off('file/event', callback)
},
}
}

Expand Down
4 changes: 1 addition & 3 deletions app/gui2/shared/languageServerTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,7 @@ export namespace response {
contentRoots: ContentRoot[]
}

export interface FileContents {
contents: TextFileContents
}
export interface FileContents extends TextFileContents {}

export interface FileExists {
exists: boolean
Expand Down
118 changes: 118 additions & 0 deletions app/gui2/shared/retry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { wait } from 'lib0/promise'

export interface BackoffOptions<E> {
maxRetries?: number
retryDelay?: number
retryDelayMultiplier?: number
retryDelayMax?: number
/** Called when the promise throws an error, and the next retry is about to be attempted.
* When this function returns `false`, the backoff is immediately aborted. When this function
* is not provided, the backoff will always continue until the maximum number of retries
* is reached. * */
onBeforeRetry?: (
error: E,
retryCount: number,
maxRetries: number,
delay: number,
) => boolean | void
/** Called right before returning. */
onSuccess?: (retryCount: number) => void
/** Called after the final retry, right before throwing an error.
* Note that `onBeforeRetry` is *not* called on the final retry, as there is nothing after the
* final retry. */
onFailure?: (error: E, retryCount: number) => void
}

const defaultBackoffOptions: Required<BackoffOptions<unknown>> = {
maxRetries: 3,
retryDelay: 1000,
retryDelayMultiplier: 2,
retryDelayMax: 10000,
onBeforeRetry: () => {},
onSuccess: () => {},
onFailure: () => {},
}

/** Retry a failing promise function with exponential backoff. */
export async function exponentialBackoff<T, E>(
f: () => Promise<T>,
backoffOptions?: BackoffOptions<E>,
): Promise<T> {
const {
maxRetries,
retryDelay,
retryDelayMultiplier,
retryDelayMax,
onBeforeRetry,
onSuccess,
onFailure,
} = {
...defaultBackoffOptions,
...backoffOptions,
}
for (
let retries = 0, delay = retryDelay;
;
retries += 1, delay = Math.min(retryDelayMax, delay * retryDelayMultiplier)
) {
try {
const result = await f()
onSuccess(retries)
return result
} catch (error) {
if (retries >= maxRetries) {
onFailure(error as E, retries)
throw error
}
if (onBeforeRetry(error as E, retries, maxRetries, delay) === false) throw error
await wait(delay)
}
}
}

export function defaultOnBeforeRetry(
description: string,
): NonNullable<BackoffOptions<any>['onBeforeRetry']> {
return (error, retryCount, maxRetries, delay) => {
console.error(
'Could not ' +
description +
` (${retryCount}/${maxRetries} retries), retrying after ${delay}ms...`,
)
console.error(error)
}
}

export function defaultOnFailure(
description: string,
): NonNullable<BackoffOptions<any>['onFailure']> {
return (error, retryCount) => {
console.error(
'Could not ' + description + ` (${retryCount}/${retryCount} retries), throwing error.`,
)
console.error(error)
}
}

export function defaultOnSuccess(
description: string,
): NonNullable<BackoffOptions<any>['onSuccess']> {
return (retryCount) => {
if (retryCount === 0) return
console.info(
'Successfully ' +
description +
` after ${retryCount} ${retryCount === 1 ? 'failure' : 'failures'}.`,
)
}
}

/** @param successDescription Should be in past tense, without an initial capital letter.
* @param errorDescription Should be in present tense, without an initial capital letter. */
export function printingCallbacks(successDescription: string, errorDescription: string) {
return {
onBeforeRetry: defaultOnBeforeRetry(errorDescription),
onSuccess: defaultOnSuccess(successDescription),
onFailure: defaultOnFailure(errorDescription),
} satisfies BackoffOptions<unknown>
}
Loading

0 comments on commit cbf7248

Please sign in to comment.