Skip to content

Commit

Permalink
Fix reconnecting after hibernation on Windows (#11739)
Browse files Browse the repository at this point in the history
Fixes #11716

The previoud implementation of restoring execution context assumed, that any synchronization in progress on connection close will fail - but actually if the synchronization just waited for executionContext/create, it might be successfull and effective after reconnecting.

Also, show "Language Server connection lost" when the actual language server connection is lost, and dismiss the message on reconnect.

# Important Notes
The hibernation still does not work properly - connection to Project Manager is not restored.
  • Loading branch information
farmaazon authored Dec 3, 2024
1 parent 4d2e44c commit 7a3f34d
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 35 deletions.
11 changes: 9 additions & 2 deletions app/gui/src/project-view/components/GraphEditor/toasts.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useEvent } from '@/composables/events'
import { type ProjectStore } from '@/stores/project'
import { useToast } from '@/util/toast'
import { onScopeDispose } from 'vue'

/**
* A composable which sets up several toasts for project management, and creates one for message
Expand All @@ -16,9 +16,16 @@ export function useGraphEditorToasts(projectStore: ProjectStore) {
toastStartup.show('Initializing the project. This can take up to one minute.')
projectStore.firstExecution.then(toastStartup.dismiss)

useEvent(document, 'project-manager-loading-failed', () =>
const offTransportClosed = projectStore.lsRpcConnection.on('transport/closed', () =>
toastConnectionLost.show('Lost connection to Language Server.'),
)
const offTransportConnected = projectStore.lsRpcConnection.on('transport/connected', () =>
toastConnectionLost.dismiss(),
)
onScopeDispose(() => {
offTransportClosed()
offTransportConnected()
})

projectStore.lsRpcConnection.client.onError((e) =>
toastLspError.show(`Language server error: ${e}`),
Expand Down
110 changes: 80 additions & 30 deletions app/gui/src/project-view/stores/project/executionContext.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { assert } from '@/util/assert'
import { findIndexOpt } from '@/util/data/array'
import { isSome, type Opt } from '@/util/data/opt'
import { Err, Ok, type Result } from '@/util/data/result'
import { Err, Ok, ResultError, type Result } from '@/util/data/result'
import { AsyncQueue, type AbortScope } from '@/util/net'
import {
qnReplaceProjectName,
Expand All @@ -12,7 +13,12 @@ import * as array from 'lib0/array'
import { ObservableV2 } from 'lib0/observable'
import * as random from 'lib0/random'
import { reactive } from 'vue'
import type { LanguageServer } from 'ydoc-shared/languageServer'
import {
ErrorCode,
LsRpcError,
RemoteRpcError,
type LanguageServer,
} from 'ydoc-shared/languageServer'
import {
methodPointerEquals,
stackItemsEqual,
Expand Down Expand Up @@ -97,6 +103,7 @@ type ExecutionContextNotification = {
enum SyncStatus {
NOT_SYNCED,
QUEUED,
CREATING,
SYNCING,
SYNCED,
}
Expand Down Expand Up @@ -163,13 +170,20 @@ export class ExecutionContext extends ObservableV2<ExecutionContextNotification>
// Connection closed: the created execution context is no longer available
// There is no point in any scheduled action until resynchronization
this.queue.clear()
this.syncStatus = SyncStatus.NOT_SYNCED
this.queue.pushTask(() => {
this.clearScheduled = false
this.sync()
return Promise.resolve({ status: 'not-created' })
})
this.clearScheduled = true
// If syncing is at the first step (creating missing execution context), it is
// effectively waiting for reconnection to recreate the execution context.
// We should not clear it's outcome, as it's likely the context will be created after
// reconnection (so it's valid).
if (this.syncStatus !== SyncStatus.CREATING) {
// In other cases, any created context is destroyed after losing connection.
// The status should be cleared to 'not-created'.
this.queue.pushTask(() => {
this.clearScheduled = false
this.sync()
return Promise.resolve({ status: 'not-created' })
})
this.clearScheduled = true
}
})
this.lsRpc.on('refactoring/projectRenamed', ({ oldNormalizedName, newNormalizedName }) => {
const newIdent = tryIdentifier(newNormalizedName)
Expand Down Expand Up @@ -327,7 +341,12 @@ export class ExecutionContext extends ObservableV2<ExecutionContextNotification>
}

private sync() {
if (this.syncStatus === SyncStatus.QUEUED || this.abort.signal.aborted) return
if (
this.syncStatus === SyncStatus.QUEUED ||
this.syncStatus === SyncStatus.CREATING ||
this.abort.signal.aborted
)
return
this.syncStatus = SyncStatus.QUEUED
this.queue.pushTask(this.syncTask())
}
Expand All @@ -346,15 +365,10 @@ export class ExecutionContext extends ObservableV2<ExecutionContextNotification>

private syncTask() {
return async (state: ExecutionContextState) => {
this.syncStatus = SyncStatus.SYNCING
if (this.abort.signal.aborted) return state
let newState = { ...state }

const create = () => {
const ensureCreated = () => {
if (newState.status === 'created') return Ok()
// if (newState.status === 'broken') {
// this.withBackoff(() => this.lsRpc.destroyExecutionContext(this.id), 'Failed to destroy broken execution context')
// }
return this.withBackoff(async () => {
const result = await this.lsRpc.createExecutionContext(this.id)
if (!result.ok) return result
Expand Down Expand Up @@ -487,25 +501,61 @@ export class ExecutionContext extends ObservableV2<ExecutionContextNotification>
.map((result) => (result.status === 'rejected' ? result.reason : null))
.filter(isSome)
if (errors.length > 0) {
console.error('Failed to synchronize visualizations:', errors)
const result = Err(`Failed to synchronize visualizations: ${errors}`)
result.error.log()
return result
}
return Ok()
}

const handleError = (error: ResultError): ExecutionContextState => {
// If error tells us that the execution context is missing, we schedule
// another sync to re-create it, and set proper state.
if (
error.payload instanceof LsRpcError &&
error.payload.cause instanceof RemoteRpcError &&
error.payload.cause.code === ErrorCode.CONTEXT_NOT_FOUND
) {
this.sync()
return { status: 'not-created' }
} else {
return newState
}
}

const createResult = await create()
if (!createResult.ok) return newState
const syncStackResult = await syncStack()
if (!syncStackResult.ok) return newState
const syncEnvResult = await syncEnvironment()
if (!syncEnvResult.ok) return newState
this.emit('newVisualizationConfiguration', [new Set(this.visualizationConfigs.keys())])
await syncVisualizations()
this.emit('visualizationsConfigured', [
new Set(state.status === 'created' ? state.visualizations.keys() : []),
])
if (this.syncStatus === SyncStatus.SYNCING) {
this.syncStatus = SyncStatus.CREATING
try {
if (this.abort.signal.aborted) return newState
const createResult = await ensureCreated()
if (!createResult.ok) return newState

DEV: assert(this.syncStatus === SyncStatus.CREATING)
this.syncStatus = SyncStatus.SYNCING

const syncStackResult = await syncStack()
if (!syncStackResult.ok) return handleError(syncStackResult.error)
if (this.syncStatus !== SyncStatus.SYNCING || this.clearScheduled) return newState

const syncEnvResult = await syncEnvironment()
if (!syncEnvResult.ok) return handleError(syncEnvResult.error)
if (this.syncStatus !== SyncStatus.SYNCING || this.clearScheduled) return newState

this.emit('newVisualizationConfiguration', [new Set(this.visualizationConfigs.keys())])
const syncVisResult = await syncVisualizations()
this.emit('visualizationsConfigured', [
new Set(state.status === 'created' ? state.visualizations.keys() : []),
])
if (!syncVisResult.ok) return handleError(syncVisResult.error)
if (this.syncStatus !== SyncStatus.SYNCING || this.clearScheduled) return newState

this.syncStatus = SyncStatus.SYNCED
return newState
} finally {
// On any exception or early return we assme we're not fully synced.
if (this.syncStatus === SyncStatus.SYNCING || this.syncStatus === SyncStatus.CREATING) {
this.syncStatus = SyncStatus.NOT_SYNCED
}
}
return newState
}
}
}
2 changes: 1 addition & 1 deletion app/ydoc-server/src/languageServerSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
const reloading = this.ls.closeTextFile(this.path).then(async closing => {
if (!closing.ok) closing.error.log('Could not close file after write error:')
return exponentialBackoff(
async () => {
async (): Promise<Result<response.OpenTextFile>> => {
const result = await this.ls.openTextFile(this.path)
if (!result.ok) return result
if (!result.value.writeCapability) {
Expand Down
4 changes: 2 additions & 2 deletions app/ydoc-shared/src/util/data/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export function Ok<T>(data?: T): Result<T | undefined, never> {
}

/** Constructor of error {@link Result}. */
export function Err<E>(error: E): Result<never, E> {
return { ok: false, error: new ResultError(error) }
export function Err<E>(error: E) {
return { ok: false, error: new ResultError(error) } as const satisfies Result<never, E>
}

/** Helper function for converting optional value to {@link Result}. */
Expand Down

0 comments on commit 7a3f34d

Please sign in to comment.