Skip to content
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.

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

Fixes for Language Server sync server #8514

Merged
merged 17 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
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),
)
50 changes: 27 additions & 23 deletions app/gui2/shared/languageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ export class LanguageServer extends ObservableV2<Notifications> {
console.log(`LS [${uuid}] ${method}:`)
console.dir(params)
}
return await this.client.request({ method, params }, RPC_TIMEOUT_MS)
const ret = await this.client.request({ method, params }, RPC_TIMEOUT_MS)
return ret
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
const remoteError = RemoteRpcErrorSchema.safeParse(e)
if (remoteError.success) {
Expand Down Expand Up @@ -167,8 +168,8 @@ export class LanguageServer extends ObservableV2<Notifications> {
}

/** [Documentation](https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#textopenfile) */
openTextFile(path: Path): Promise<response.OpenTextFile> {
return this.request('text/openFile', { path })
openTextFile(path: Path, meta?: string): Promise<response.OpenTextFile> {
return this.request('text/openFile', { path, meta })
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
}

/** [Documentation](https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#textclosefile) */
Expand Down Expand Up @@ -402,27 +403,30 @@ 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(() => 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)
if (!running) return
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
})(),
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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This was the FileReadResult type, but improperly named. According to docs it does have a nested contents field. So it was probably allright, bar the incorrect name.

https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#result-5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually i discovered this was incorrect, when i was implementing this:

case LsSyncState.Synchronized: {
this.withState(LsSyncState.Reloading, async () => {
const promise = Promise.all([
this.ls.readFile(this.path),
this.ls.fileChecksum(this.path),
])
this.setLastAction(promise)
const [contents, checksum] = await promise
this.syncFileContents(contents.contents, checksum.checksum)
})
return
}

on that note... i'm not 100% sure about this. it doesn't affect the fix if it's removed - it's entirely possible that I've misunderstood what reload is supposed to do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that there is a FileContents type: https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#filecontents

also note that *Result types were basically only added so that the Result interfaces are valid TS code - in GUI2 these should correspond to response.*

contents: TextFileContents
}
export interface FileContents extends TextFileContents {}

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

export interface BackoffOptions<E> {
maxRetries?: number
retryDelay?: number
retryDelayMultiplier?: number
retryDelayMax?: number
/**
* Called when the promise return an error result, 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 right before throwing an error. Note that `onBeforeRetry` is called for all tries
* before the last one. */
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
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 onBeforeRetry(
description: string,
): NonNullable<BackoffOptions<any>['onBeforeRetry']> {
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
return (error, retryCount, maxRetries, delay) => {
console.error(
'Could not ' +
description +
` (${retryCount}/${maxRetries} retries), retrying after ${delay}ms...`,
)
console.error(error)
}
}

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

export function onSuccess(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 successDescription Should be in present tense, without an initial capital letter. */
export function exponentialBackoffMessages(successDescription: string, errorDescription: string) {
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
return {
onBeforeRetry: onBeforeRetry(errorDescription),
onSuccess: onSuccess(successDescription),
onFailure: onFailure(errorDescription),
} satisfies BackoffOptions<unknown>
}
Loading
Loading