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 3 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
16 changes: 7 additions & 9 deletions app/gui2/shared/languageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,15 @@ export class LanguageServer extends ObservableV2<Notifications> {
console.log(`LS [${uuid}] ${method}:`)
console.dir(params)
}
const ret = await this.client.request({ method, params }, RPC_TIMEOUT_MS)
return ret
} catch (e) {
const remoteError = RemoteRpcErrorSchema.safeParse(e)
return await this.client.request({ method, params }, RPC_TIMEOUT_MS)
} 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 @@ -407,7 +406,7 @@ export class LanguageServer extends ObservableV2<Notifications> {
return {
promise: (async () => {
self.on('file/event', callback)
await retry(() => self.acquireReceivesTreeUpdates({ rootId, segments }))
await retry(async () => running && self.acquireReceivesTreeUpdates({ rootId, segments }))
await walkFs(self, { rootId, segments }, (type, path) => {
if (
!running ||
Expand All @@ -421,7 +420,6 @@ export class LanguageServer extends ObservableV2<Notifications> {
kind: 'Added',
})
})
if (!running) return
})(),
unsubscribe() {
running = false
Expand Down
25 changes: 15 additions & 10 deletions app/gui2/shared/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ export interface BackoffOptions<E> {
) => 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. */
/** Called on the final retry, right before throwing an error.
* Note that `onBeforeRetry` is *not* called on the final retry, as there is nothing after the
somebody1234 marked this conversation as resolved.
Show resolved Hide resolved
* final retry. */
onFailure?: (error: E, retryCount: number) => void
}

Expand Down Expand Up @@ -70,7 +71,7 @@ export async function exponentialBackoff<T, E>(
}
}

export function onBeforeRetry(
export function defaultOnBeforeRetry(
description: string,
): NonNullable<BackoffOptions<any>['onBeforeRetry']> {
return (error, retryCount, maxRetries, delay) => {
Expand All @@ -83,7 +84,9 @@ export function onBeforeRetry(
}
}

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

export function onSuccess(description: string): NonNullable<BackoffOptions<any>['onSuccess']> {
export function defaultOnSuccess(
description: string,
): NonNullable<BackoffOptions<any>['onSuccess']> {
return (retryCount) => {
if (retryCount === 0) return
console.info(
Expand All @@ -104,11 +109,11 @@ export function onSuccess(description: string): NonNullable<BackoffOptions<any>[
}

/** @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) {
* @param errorDescription Should be in present tense, without an initial capital letter. */
export function printingCallbacks(successDescription: string, errorDescription: string) {
return {
onBeforeRetry: onBeforeRetry(errorDescription),
onSuccess: onSuccess(successDescription),
onFailure: onFailure(errorDescription),
onBeforeRetry: defaultOnBeforeRetry(errorDescription),
onSuccess: defaultOnSuccess(successDescription),
onFailure: defaultOnFailure(errorDescription),
} satisfies BackoffOptions<unknown>
}
12 changes: 6 additions & 6 deletions app/gui2/src/components/GraphEditor/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,11 @@ export class Uploader {
private async pickUniqueName(suggestedName: string): Promise<string> {
const files = await this.rpc.listFiles(this.dataDirPath())
const existingNames = new Set(files.paths.map((path) => path.name))
const { name, extension = '' } = splitFilename(suggestedName)
const { stem, extension = '' } = splitFilename(suggestedName)
let candidate = suggestedName
let num = 1
while (existingNames.has(candidate)) {
candidate = `${name}_${num}.${extension}`
candidate = `${stem}_${num}.${extension}`
num += 1
}
return candidate
Expand All @@ -195,12 +195,12 @@ export class Uploader {
/**
* Split filename into stem and (optional) extension.
*/
function splitFilename(fileName: string): { name: string; extension?: string } {
function splitFilename(fileName: string): { stem: string; extension?: string } {
const dotIndex = fileName.lastIndexOf('.')
if (dotIndex !== -1 && dotIndex !== 0) {
const name = fileName.substring(0, dotIndex)
const stem = fileName.substring(0, dotIndex)
const extension = fileName.substring(dotIndex + 1)
return { name, extension }
return { stem, extension }
}
return { name: fileName }
return { stem: fileName }
}
17 changes: 14 additions & 3 deletions app/gui2/src/stores/visualization/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { isUrlString } from '@/util/data/urlString'
import { isIconName } from '@/util/iconName'
import { rpcWithRetries } from '@/util/net'
import { defineStore } from 'pinia'
import { ErrorCode, LsRpcError, RemoteRpcError } from 'shared/languageServer'
import type { Event as LSEvent, VisualizationConfiguration } from 'shared/languageServerTypes'
import type { ExprId, VisualizationIdentifier } from 'shared/yjsModel'
import { computed, reactive } from 'vue'
Expand Down Expand Up @@ -216,9 +217,19 @@ export const useVisualizationStore = defineStore('visualization', () => {
try {
await ls.watchFiles(projectRoot, [customVisualizationsDirectory], onFileEvent, rpcWithRetries)
.promise
} catch {
// Ignored. It is very likely that the `visualizations/` directory is not present, however
// there would be a race condition if the existence of `visualizations` is checked first.
} catch (error) {
if (
error instanceof LsRpcError &&
error.cause instanceof RemoteRpcError &&
error.cause.code === ErrorCode.FILE_NOT_FOUND
) {
console.info(
"'visualizations/' folder not found in project directory. " +
"If you have custom visualizations, please put them under 'visualizations/'.",
)
} else {
throw error
}
}
})

Expand Down
21 changes: 13 additions & 8 deletions app/gui2/ydoc-server/languageServerSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ export class LanguageServerSession {
docs: Map<string, WSSharedDoc>
retainCount: number
url: string
client!: Client
ls!: LanguageServer
client: Client
ls: LanguageServer
connection: response.InitProtocolConnection | undefined
model: DistributedProject
projectRootId: Uuid | null
Expand All @@ -72,7 +72,9 @@ export class LanguageServerSession {
if (!persistence) continue
}
})
this.restartClient()
const { client, ls } = this.setupClient()
this.client = client
this.ls = ls
}

static sessions = new Map<string, LanguageServerSession>()
Expand All @@ -87,13 +89,15 @@ export class LanguageServerSession {
}

private restartClient() {
// `this.client` WILL be undefined on the first call.
this.client?.close()
this.client.close()
this.ls.destroy()
this.connection = undefined
this.setupClient()
}

private setupClient() {
this.client = createOpenRPCClient(this.url)
// `this.ls` WILL be undefined on the first call.
this.ls?.destroy()
this.ls = new LanguageServer(this.client)
this.connection = undefined
this.ls.on('file/event', async (event) => {
if (DEBUG_LOG_SYNC) {
console.log('file/event', event)
Expand Down Expand Up @@ -150,6 +154,7 @@ export class LanguageServerSession {
exponentialBackoffMessages('restarted RPC client', 'restart RPC client'),
)
})
return { client: this.client, ls: this.ls }
}

private assertProjectRoot(): asserts this is { projectRootId: Uuid } {
Expand Down
Loading