From 71491e03468795404751d2edfe58ce78714a5ed1 Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Thu, 25 Aug 2022 00:14:51 +0200 Subject: refactor(frontend): xtext update improvements --- subprojects/frontend/src/editor/findOccurrences.ts | 28 ++- .../frontend/src/utils/ConditionVariable.ts | 10 +- subprojects/frontend/src/utils/PendingTask.ts | 11 +- subprojects/frontend/src/utils/Timer.ts | 20 +-- .../frontend/src/xtext/ContentAssistService.ts | 39 +++-- .../frontend/src/xtext/OccurrencesService.ts | 34 +++- subprojects/frontend/src/xtext/UpdateService.ts | 193 +++++++++++++-------- .../frontend/src/xtext/XtextWebSocketClient.ts | 71 ++++---- 8 files changed, 252 insertions(+), 154 deletions(-) (limited to 'subprojects/frontend') diff --git a/subprojects/frontend/src/editor/findOccurrences.ts b/subprojects/frontend/src/editor/findOccurrences.ts index d7aae8d1..08c078c2 100644 --- a/subprojects/frontend/src/editor/findOccurrences.ts +++ b/subprojects/frontend/src/editor/findOccurrences.ts @@ -1,4 +1,9 @@ -import { type Range, RangeSet, type TransactionSpec } from '@codemirror/state'; +import { + type Range, + RangeSet, + type TransactionSpec, + type EditorState, +} from '@codemirror/state'; import { Decoration } from '@codemirror/view'; import defineDecorationSetExtension from './defineDecorationSetExtension'; @@ -34,4 +39,25 @@ export function setOccurrences( return setOccurrencesInteral(rangeSet); } +export function isCursorWithinOccurence(state: EditorState): boolean { + const occurrences = state.field(findOccurrences, false); + if (occurrences === undefined) { + return false; + } + const { + selection: { + main: { from, to }, + }, + } = state; + let found = false; + occurrences.between(from, to, (decorationFrom, decorationTo) => { + if (decorationFrom <= from && to <= decorationTo) { + found = true; + return false; + } + return undefined; + }); + return found; +} + export default findOccurrences; diff --git a/subprojects/frontend/src/utils/ConditionVariable.ts b/subprojects/frontend/src/utils/ConditionVariable.ts index c8fae9e8..1d3431f7 100644 --- a/subprojects/frontend/src/utils/ConditionVariable.ts +++ b/subprojects/frontend/src/utils/ConditionVariable.ts @@ -6,22 +6,22 @@ const log = getLogger('utils.ConditionVariable'); export type Condition = () => boolean; export default class ConditionVariable { - condition: Condition; + private readonly condition: Condition; - defaultTimeout: number; + private readonly defaultTimeout: number; - listeners: PendingTask[] = []; + private listeners: PendingTask[] = []; constructor(condition: Condition, defaultTimeout = 0) { this.condition = condition; this.defaultTimeout = defaultTimeout; } - async waitFor(timeoutMs: number | null = null): Promise { + async waitFor(timeoutMs?: number | undefined): Promise { if (this.condition()) { return; } - const timeoutOrDefault = timeoutMs || this.defaultTimeout; + const timeoutOrDefault = timeoutMs ?? this.defaultTimeout; let nowMs = Date.now(); const endMs = nowMs + timeoutOrDefault; while (!this.condition() && nowMs < endMs) { diff --git a/subprojects/frontend/src/utils/PendingTask.ts b/subprojects/frontend/src/utils/PendingTask.ts index 086993d4..3976bdf9 100644 --- a/subprojects/frontend/src/utils/PendingTask.ts +++ b/subprojects/frontend/src/utils/PendingTask.ts @@ -9,13 +9,13 @@ export default class PendingTask { private resolved = false; - private timeout: number | null; + private timeout: number | undefined; constructor( resolveCallback: (value: T) => void, rejectCallback: (reason?: unknown) => void, - timeoutMs?: number, - timeoutCallback?: () => void, + timeoutMs?: number | undefined, + timeoutCallback?: () => void | undefined, ) { this.resolveCallback = resolveCallback; this.rejectCallback = rejectCallback; @@ -28,8 +28,6 @@ export default class PendingTask { } } }, timeoutMs); - } else { - this.timeout = null; } } @@ -53,8 +51,9 @@ export default class PendingTask { private markResolved() { this.resolved = true; - if (this.timeout !== null) { + if (this.timeout !== undefined) { clearTimeout(this.timeout); + this.timeout = undefined; } } } diff --git a/subprojects/frontend/src/utils/Timer.ts b/subprojects/frontend/src/utils/Timer.ts index 14e9eb81..4bb1bb9c 100644 --- a/subprojects/frontend/src/utils/Timer.ts +++ b/subprojects/frontend/src/utils/Timer.ts @@ -1,33 +1,33 @@ export default class Timer { - readonly callback: () => void; + private readonly callback: () => void; - readonly defaultTimeout: number; + private readonly defaultTimeout: number; - timeout: number | null = null; + private timeout: number | undefined; constructor(callback: () => void, defaultTimeout = 0) { this.callback = () => { - this.timeout = null; + this.timeout = undefined; callback(); }; this.defaultTimeout = defaultTimeout; } - schedule(timeout: number | null = null): void { - if (this.timeout === null) { - this.timeout = setTimeout(this.callback, timeout || this.defaultTimeout); + schedule(timeout?: number | undefined): void { + if (this.timeout === undefined) { + this.timeout = setTimeout(this.callback, timeout ?? this.defaultTimeout); } } - reschedule(timeout: number | null = null): void { + reschedule(timeout?: number | undefined): void { this.cancel(); this.schedule(timeout); } cancel(): void { - if (this.timeout !== null) { + if (this.timeout !== undefined) { clearTimeout(this.timeout); - this.timeout = null; + this.timeout = undefined; } } } diff --git a/subprojects/frontend/src/xtext/ContentAssistService.ts b/subprojects/frontend/src/xtext/ContentAssistService.ts index dce2a902..39042812 100644 --- a/subprojects/frontend/src/xtext/ContentAssistService.ts +++ b/subprojects/frontend/src/xtext/ContentAssistService.ts @@ -31,15 +31,12 @@ interface IFoundToken { text: string; } -function findToken({ pos, state }: CompletionContext): IFoundToken | null { +function findToken({ pos, state }: CompletionContext): IFoundToken | undefined { const token = syntaxTree(state).resolveInner(pos, -1); - if (token === null) { - return null; - } if (token.firstChild !== null) { // We only autocomplete terminal nodes. If the current node is nonterminal, - // returning `null` makes us autocomplete with the empty prefix instead. - return null; + // returning `undefined` makes us autocomplete with the empty prefix instead. + return undefined; } return { from: token.from, @@ -50,11 +47,13 @@ function findToken({ pos, state }: CompletionContext): IFoundToken | null { } function shouldCompleteImplicitly( - token: IFoundToken | null, + token: IFoundToken | undefined, context: CompletionContext, ): boolean { return ( - token !== null && token.implicitCompletion && context.pos - token.from >= 2 + token !== undefined && + token.implicitCompletion && + context.pos - token.from >= 2 ); } @@ -107,7 +106,7 @@ function createCompletion(entry: ContentAssistEntry): Completion { export default class ContentAssistService { private readonly updateService: UpdateService; - private lastCompletion: CompletionResult | null = null; + private lastCompletion: CompletionResult | undefined; constructor(updateService: UpdateService) { this.updateService = updateService; @@ -115,7 +114,7 @@ export default class ContentAssistService { onTransaction(transaction: Transaction): void { if (this.shouldInvalidateCachedCompletion(transaction)) { - this.lastCompletion = null; + this.lastCompletion = undefined; } } @@ -129,7 +128,7 @@ export default class ContentAssistService { } let range: { from: number; to: number }; let prefix = ''; - if (tokenBefore === null) { + if (tokenBefore === undefined) { range = { from: context.pos, to: context.pos, @@ -146,14 +145,18 @@ export default class ContentAssistService { } } if (!context.explicit && this.shouldReturnCachedCompletion(tokenBefore)) { + if (this.lastCompletion === undefined) { + throw new Error( + 'There is no cached completion, but we want to return it', + ); + } log.trace('Returning cached completion result'); - // Postcondition of `shouldReturnCachedCompletion`: `lastCompletion !== null` return { - ...(this.lastCompletion as CompletionResult), + ...this.lastCompletion, ...range, }; } - this.lastCompletion = null; + this.lastCompletion = undefined; const entries = await this.updateService.fetchContentAssist( { resource: this.updateService.resourceName, @@ -188,9 +191,9 @@ export default class ContentAssistService { } private shouldReturnCachedCompletion( - token: { from: number; to: number; text: string } | null, + token: { from: number; to: number; text: string } | undefined, ): boolean { - if (token === null || this.lastCompletion === null) { + if (token === undefined || this.lastCompletion === undefined) { return false; } const { from, to, text } = token; @@ -211,11 +214,11 @@ export default class ContentAssistService { } private shouldInvalidateCachedCompletion(transaction: Transaction): boolean { - if (!transaction.docChanged || this.lastCompletion === null) { + if (!transaction.docChanged || this.lastCompletion === undefined) { return false; } const { from: lastFrom, to: lastTo } = this.lastCompletion; - if (!lastTo) { + if (lastTo === undefined) { return true; } const [transformedFrom, transformedTo] = this.mapRangeInclusive( diff --git a/subprojects/frontend/src/xtext/OccurrencesService.ts b/subprojects/frontend/src/xtext/OccurrencesService.ts index 21fe8644..35913f43 100644 --- a/subprojects/frontend/src/xtext/OccurrencesService.ts +++ b/subprojects/frontend/src/xtext/OccurrencesService.ts @@ -1,7 +1,10 @@ import { Transaction } from '@codemirror/state'; import type EditorStore from '../editor/EditorStore'; -import type { IOccurrence } from '../editor/findOccurrences'; +import { + type IOccurrence, + isCursorWithinOccurence, +} from '../editor/findOccurrences'; import Timer from '../utils/Timer'; import getLogger from '../utils/getLogger'; @@ -15,10 +18,6 @@ import { const FIND_OCCURRENCES_TIMEOUT_MS = 1000; -// Must clear occurrences asynchronously from `onTransaction`, -// because we must not emit a conflicting transaction when handling the pending transaction. -const CLEAR_OCCURRENCES_TIMEOUT_MS = 10; - const log = getLogger('xtext.OccurrencesService'); function transformOccurrences(regions: TextRegion[]): IOccurrence[] { @@ -49,7 +48,7 @@ export default class OccurrencesService { private readonly clearOccurrencesTimer = new Timer(() => { this.clearOccurrences(); - }, CLEAR_OCCURRENCES_TIMEOUT_MS); + }); constructor( store: EditorStore, @@ -63,12 +62,27 @@ export default class OccurrencesService { onTransaction(transaction: Transaction): void { if (transaction.docChanged) { + // Must clear occurrences asynchronously from `onTransaction`, + // because we must not emit a conflicting transaction when handling the pending transaction. this.clearOccurrencesTimer.schedule(); this.findOccurrencesTimer.reschedule(); + return; } - if (transaction.isUserEvent('select')) { - this.findOccurrencesTimer.reschedule(); + if (!transaction.isUserEvent('select')) { + return; } + if (this.needsOccurrences) { + if (!isCursorWithinOccurence(this.store.state)) { + this.clearOccurrencesTimer.schedule(); + this.findOccurrencesTimer.reschedule(); + } + } else { + this.clearOccurrencesTimer.schedule(); + } + } + + private get needsOccurrences(): boolean { + return this.store.state.selection.main.empty; } private handleFindOccurrences() { @@ -80,6 +94,10 @@ export default class OccurrencesService { } private async updateOccurrences() { + if (!this.needsOccurrences) { + this.clearOccurrences(); + return; + } await this.updateService.update(); const result = await this.webSocketClient.send({ resource: this.updateService.resourceName, diff --git a/subprojects/frontend/src/xtext/UpdateService.ts b/subprojects/frontend/src/xtext/UpdateService.ts index 2994b11b..f8b71160 100644 --- a/subprojects/frontend/src/xtext/UpdateService.ts +++ b/subprojects/frontend/src/xtext/UpdateService.ts @@ -27,24 +27,47 @@ const WAIT_FOR_UPDATE_TIMEOUT_MS = 1000; const log = getLogger('xtext.UpdateService'); +/** + * State effect used to override the dirty changes after a transaction. + * + * If this state effect is _not_ present in a transaction, + * the transaction will be appended to the current dirty changes. + * + * If this state effect is present, the current dirty changes will be replaced + * by the value of this effect. + */ const setDirtyChanges = StateEffect.define(); export interface IAbortSignal { aborted: boolean; } +interface StateUpdateResult { + newStateId: string; + + data: T; +} + +interface Delta { + deltaOffset: number; + + deltaReplaceLength: number; + + deltaText: string; +} + export default class UpdateService { resourceName: string; - xtextStateId: string | null = null; + xtextStateId: string | undefined; private readonly store: EditorStore; /** * The changes being synchronized to the server if a full or delta text update is running, - * `null` otherwise. + * `undefined` otherwise. */ - private pendingUpdate: ChangeSet | null = null; + private pendingUpdate: ChangeSet | undefined; /** * Local changes not yet sychronized to the server and not part of the running update, if any. @@ -54,7 +77,7 @@ export default class UpdateService { private readonly webSocketClient: XtextWebSocketClient; private readonly updatedCondition = new ConditionVariable( - () => this.pendingUpdate === null && this.xtextStateId !== null, + () => this.pendingUpdate === undefined && this.xtextStateId !== undefined, WAIT_FOR_UPDATE_TIMEOUT_MS, ); @@ -70,7 +93,7 @@ export default class UpdateService { } onReconnect(): void { - this.xtextStateId = null; + this.xtextStateId = undefined; this.updateFullText().catch((error) => { log.error('Unexpected error during initial update', error); }); @@ -82,7 +105,7 @@ export default class UpdateService { ) as StateEffect | undefined; if (setDirtyChangesEffect) { const { value } = setDirtyChangesEffect; - if (this.pendingUpdate !== null) { + if (this.pendingUpdate !== undefined) { this.pendingUpdate = ChangeSet.empty(value.length); } this.dirtyChanges = value; @@ -100,20 +123,20 @@ export default class UpdateService { * The result reflects any changes that happened since the `xtextStateId` * version was uploaded to the server. * - * @return the summary of changes since the last update + * @returns the summary of changes since the last update */ computeChangesSinceLastUpdate(): ChangeDesc { return ( - this.pendingUpdate?.composeDesc(this.dirtyChanges.desc) || + this.pendingUpdate?.composeDesc(this.dirtyChanges.desc) ?? this.dirtyChanges.desc ); } - private handleIdleUpdate() { + private handleIdleUpdate(): void { if (!this.webSocketClient.isOpen || this.dirtyChanges.empty) { return; } - if (this.pendingUpdate === null) { + if (this.pendingUpdate === undefined) { this.update().catch((error) => { log.error('Unexpected error during scheduled update', error); }); @@ -121,7 +144,7 @@ export default class UpdateService { this.idleUpdateTimer.reschedule(); } - private newEmptyChangeSet() { + private newEmptyChangeSet(): ChangeSet { return ChangeSet.of([], this.store.state.doc.length); } @@ -129,14 +152,14 @@ export default class UpdateService { await this.withUpdate(() => this.doUpdateFullText()); } - private async doUpdateFullText(): Promise<[string, void]> { + private async doUpdateFullText(): Promise> { const result = await this.webSocketClient.send({ resource: this.resourceName, serviceType: 'update', fullText: this.store.state.doc.sliceString(0), }); const { stateId } = DocumentStateResult.parse(result); - return [stateId, undefined]; + return { newStateId: stateId, data: undefined }; } /** @@ -146,12 +169,12 @@ export default class UpdateService { * Performs either an update with delta text or a full text update if needed. * If there are not local dirty changes, the promise resolves immediately. * - * @return a promise resolving when the update is completed + * @returns a promise resolving when the update is completed */ async update(): Promise { await this.prepareForDeltaUpdate(); const delta = this.computeDelta(); - if (delta === null) { + if (delta === undefined) { return; } log.trace('Editor delta', delta); @@ -164,7 +187,10 @@ export default class UpdateService { }); const parsedDocumentStateResult = DocumentStateResult.safeParse(result); if (parsedDocumentStateResult.success) { - return [parsedDocumentStateResult.data.stateId, undefined]; + return { + newStateId: parsedDocumentStateResult.data.stateId, + data: undefined, + }; } if (isConflictResult(result, 'invalidStateId')) { return this.doFallbackToUpdateFullText(); @@ -173,12 +199,12 @@ export default class UpdateService { }); } - private doFallbackToUpdateFullText() { - if (this.pendingUpdate === null) { + private doFallbackToUpdateFullText(): Promise> { + if (this.pendingUpdate === undefined) { throw new Error('Only a pending update can be extended'); } log.warn('Delta update failed, performing full text update'); - this.xtextStateId = null; + this.xtextStateId = undefined; this.pendingUpdate = this.pendingUpdate.compose(this.dirtyChanges); this.dirtyChanges = this.newEmptyChangeSet(); return this.doUpdateFullText(); @@ -193,56 +219,69 @@ export default class UpdateService { return []; } const delta = this.computeDelta(); - if (delta !== null) { + if (delta !== undefined) { log.trace('Editor delta', delta); - const entries = await this.withUpdate(async () => { - const result = await this.webSocketClient.send({ - ...params, - requiredStateId: this.xtextStateId, - ...delta, - }); - const parsedContentAssistResult = ContentAssistResult.safeParse(result); - if (parsedContentAssistResult.success) { - const { stateId, entries: resultEntries } = - parsedContentAssistResult.data; - return [stateId, resultEntries]; - } - if (isConflictResult(result, 'invalidStateId')) { - log.warn('Server state invalid during content assist'); - const [newStateId] = await this.doFallbackToUpdateFullText(); - // We must finish this state update transaction to prepare for any push events - // before querying for content assist, so we just return `null` and will query - // the content assist service later. - return [newStateId, null]; - } - throw parsedContentAssistResult.error; - }); - if (entries !== null) { - return entries; + // Try to fetch while also performing a delta update. + const fetchUpdateEntries = await this.withUpdate(() => + this.doFetchContentAssistWithDelta(params, delta), + ); + if (fetchUpdateEntries !== undefined) { + return fetchUpdateEntries; } if (signal.aborted) { return []; } } - // Poscondition of `prepareForDeltaUpdate`: `xtextStateId !== null` - return this.doFetchContentAssist(params, this.xtextStateId as string); + if (this.xtextStateId === undefined) { + throw new Error('failed to obtain Xtext state id'); + } + return this.doFetchContentAssistFetchOnly(params, this.xtextStateId); } - private async doFetchContentAssist( + private async doFetchContentAssistWithDelta( params: Record, - expectedStateId: string, - ) { - const result = await this.webSocketClient.send({ + delta: Delta, + ): Promise> { + const fetchUpdateResult = await this.webSocketClient.send({ + ...params, + requiredStateId: this.xtextStateId, + ...delta, + }); + const parsedContentAssistResult = + ContentAssistResult.safeParse(fetchUpdateResult); + if (parsedContentAssistResult.success) { + const { stateId, entries: resultEntries } = + parsedContentAssistResult.data; + return { newStateId: stateId, data: resultEntries }; + } + if (isConflictResult(fetchUpdateResult, 'invalidStateId')) { + log.warn('Server state invalid during content assist'); + const { newStateId } = await this.doFallbackToUpdateFullText(); + // We must finish this state update transaction to prepare for any push events + // before querying for content assist, so we just return `undefined` and will query + // the content assist service later. + return { newStateId, data: undefined }; + } + throw parsedContentAssistResult.error; + } + + private async doFetchContentAssistFetchOnly( + params: Record, + requiredStateId: string, + ): Promise { + // Fallback to fetching without a delta update. + const fetchOnlyResult = await this.webSocketClient.send({ ...params, - requiredStateId: expectedStateId, + requiredStateId: this.xtextStateId, }); - const { stateId, entries } = ContentAssistResult.parse(result); - if (stateId !== expectedStateId) { + const { stateId, entries: fetchOnlyEntries } = + ContentAssistResult.parse(fetchOnlyResult); + if (stateId !== requiredStateId) { throw new Error( - `Unexpected state id, expected: ${expectedStateId} got: ${stateId}`, + `Unexpected state id, expected: ${requiredStateId} got: ${stateId}`, ); } - return entries; + return fetchOnlyEntries; } async formatText(): Promise { @@ -253,7 +292,7 @@ export default class UpdateService { to = this.store.state.doc.length; } log.debug('Formatting from', from, 'to', to); - await this.withUpdate(async () => { + await this.withUpdate(async () => { const result = await this.webSocketClient.send({ resource: this.resourceName, serviceType: 'format', @@ -266,13 +305,13 @@ export default class UpdateService { to, insert: formattedText, }); - return [stateId, null]; + return { newStateId: stateId, data: undefined }; }); } - private computeDelta() { + private computeDelta(): Delta | undefined { if (this.dirtyChanges.empty) { - return null; + return undefined; } let minFromA = Number.MAX_SAFE_INTEGER; let maxToA = 0; @@ -291,15 +330,17 @@ export default class UpdateService { }; } - private applyBeforeDirtyChanges(changeSpec: ChangeSpec) { + private applyBeforeDirtyChanges(changeSpec: ChangeSpec): void { const pendingChanges = - this.pendingUpdate?.compose(this.dirtyChanges) || this.dirtyChanges; + this.pendingUpdate?.compose(this.dirtyChanges) ?? this.dirtyChanges; const revertChanges = pendingChanges.invert(this.store.state.doc); const applyBefore = ChangeSet.of(changeSpec, revertChanges.newLength); const redoChanges = pendingChanges.map(applyBefore.desc); const changeSet = revertChanges.compose(applyBefore).compose(redoChanges); this.store.dispatch({ changes: changeSet, + // Keep the current set of dirty changes (but update them according the re-formatting) + // and to not add the formatting the dirty changes. effects: [setDirtyChanges.of(redoChanges)], }); } @@ -318,37 +359,35 @@ export default class UpdateService { * to ensure that the local `stateId` is updated likewise to be able to handle * push messages referring to the new `stateId` from the server. * If additional work is needed to compute the second value in some cases, - * use `T | null` instead of `T` as a return type and signal the need for additional - * computations by returning `null`. Thus additional computations can be performed + * use `T | undefined` instead of `T` as a return type and signal the need for additional + * computations by returning `undefined`. Thus additional computations can be performed * outside of the critical section. * * @param callback the asynchronous callback that updates the server state - * @return a promise resolving to the second value returned by `callback` + * @returns a promise resolving to the second value returned by `callback` */ private async withUpdate( - callback: () => Promise<[string, T]>, + callback: () => Promise>, ): Promise { - if (this.pendingUpdate !== null) { + if (this.pendingUpdate !== undefined) { throw new Error('Another update is pending, will not perform update'); } this.pendingUpdate = this.dirtyChanges; this.dirtyChanges = this.newEmptyChangeSet(); - let newStateId: string | null = null; try { - let result: T; - [newStateId, result] = await callback(); + const { newStateId, data } = await callback(); this.xtextStateId = newStateId; - this.pendingUpdate = null; + this.pendingUpdate = undefined; this.updatedCondition.notifyAll(); - return result; + return data; } catch (e) { log.error('Error while update', e); - if (this.pendingUpdate === null) { + if (this.pendingUpdate === undefined) { log.error('pendingUpdate was cleared during update'); } else { this.dirtyChanges = this.pendingUpdate.compose(this.dirtyChanges); } - this.pendingUpdate = null; + this.pendingUpdate = undefined; this.webSocketClient.forceReconnectOnError(); this.updatedCondition.rejectAll(e); throw e; @@ -357,16 +396,16 @@ export default class UpdateService { /** * Ensures that there is some state available on the server (`xtextStateId`) - * and that there is not pending update. + * and that there is no pending update. * * After this function resolves, a delta text update is possible. * - * @return a promise resolving when there is a valid state id but no pending update + * @returns a promise resolving when there is a valid state id but no pending update */ - private async prepareForDeltaUpdate() { + private async prepareForDeltaUpdate(): Promise { // If no update is pending, but the full text hasn't been uploaded to the server yet, // we must start a full text upload. - if (this.pendingUpdate === null && this.xtextStateId === null) { + if (this.pendingUpdate === undefined && this.xtextStateId === undefined) { await this.updateFullText(); } await this.updatedCondition.waitFor(); diff --git a/subprojects/frontend/src/xtext/XtextWebSocketClient.ts b/subprojects/frontend/src/xtext/XtextWebSocketClient.ts index ceb1f3fd..60bf6ba9 100644 --- a/subprojects/frontend/src/xtext/XtextWebSocketClient.ts +++ b/subprojects/frontend/src/xtext/XtextWebSocketClient.ts @@ -17,6 +17,8 @@ const XTEXT_SUBPROTOCOL_V1 = 'tools.refinery.language.web.xtext.v1'; const WEBSOCKET_CLOSE_OK = 1000; +const WEBSOCKET_CLOSE_GOING_AWAY = 1001; + const RECONNECT_DELAY_MS = [200, 1000, 5000, 30_000]; const MAX_RECONNECT_DELAY_MS = @@ -44,9 +46,9 @@ enum State { Opening, TabVisible, TabHiddenIdle, - TabHiddenWaiting, + TabHiddenWaitingToClose, Error, - TimedOut, + ClosedDueToInactivity, } export default class XtextWebSocketClient { @@ -86,14 +88,16 @@ export default class XtextWebSocketClient { } private get isLogicallyClosed(): boolean { - return this.state === State.Error || this.state === State.TimedOut; + return ( + this.state === State.Error || this.state === State.ClosedDueToInactivity + ); } get isOpen(): boolean { return ( this.state === State.TabVisible || this.state === State.TabHiddenIdle || - this.state === State.TabHiddenWaiting + this.state === State.TabHiddenWaitingToClose ); } @@ -134,11 +138,15 @@ export default class XtextWebSocketClient { this.handleMessage(event.data); }); this.connection.addEventListener('close', (event) => { - if ( + const closedOnRequest = this.isLogicallyClosed && event.code === WEBSOCKET_CLOSE_OK && - this.pendingRequests.size === 0 - ) { + this.pendingRequests.size === 0; + const closedOnNavigation = event.code === WEBSOCKET_CLOSE_GOING_AWAY; + if (closedOnNavigation) { + this.state = State.ClosedDueToInactivity; + } + if (closedOnRequest || closedOnNavigation) { log.info('Websocket closed'); return; } @@ -157,12 +165,12 @@ export default class XtextWebSocketClient { this.idleTimer.cancel(); if ( this.state === State.TabHiddenIdle || - this.state === State.TabHiddenWaiting + this.state === State.TabHiddenWaitingToClose ) { this.handleTabVisibleConnected(); return; } - if (this.state === State.TimedOut) { + if (this.state === State.ClosedDueToInactivity) { this.reconnect(); } } @@ -181,19 +189,19 @@ export default class XtextWebSocketClient { private handleIdleTimeout() { log.trace('Waiting for pending tasks before disconnect'); if (this.state === State.TabHiddenIdle) { - this.state = State.TabHiddenWaiting; + this.state = State.TabHiddenWaitingToClose; this.handleWaitingForDisconnect(); } } private handleWaitingForDisconnect() { - if (this.state !== State.TabHiddenWaiting) { + if (this.state !== State.TabHiddenWaitingToClose) { return; } const pending = this.pendingRequests.size; if (pending === 0) { log.info('Closing idle websocket'); - this.state = State.TimedOut; + this.state = State.ClosedDueToInactivity; this.closeConnection(1000, 'idle timeout'); return; } @@ -334,17 +342,31 @@ export default class XtextWebSocketClient { if (this.isLogicallyClosed) { return; } - this.abortPendingRequests(); - this.closeConnection(1000, 'reconnecting due to error'); - log.error('Reconnecting after delay due to error'); - this.handleErrorState(); - } - - private abortPendingRequests() { this.pendingRequests.forEach((request) => { request.reject(new Error('Websocket disconnect')); }); this.pendingRequests.clear(); + this.closeConnection(1000, 'reconnecting due to error'); + if (this.state === State.Error) { + // We are already handling this error condition. + return; + } + if ( + this.state === State.TabHiddenIdle || + this.state === State.TabHiddenWaitingToClose + ) { + log.error('Will reconned due to error once the tab becomes visible'); + this.idleTimer.cancel(); + this.state = State.ClosedDueToInactivity; + return; + } + log.error('Reconnecting after delay due to error'); + this.state = State.Error; + this.reconnectTryCount += 1; + const delay = + RECONNECT_DELAY_MS[this.reconnectTryCount - 1] ?? MAX_RECONNECT_DELAY_MS; + log.info('Reconnecting in', delay, 'ms'); + this.reconnectTimer.schedule(delay); } private closeConnection(code: number, reason: string) { @@ -355,22 +377,13 @@ export default class XtextWebSocketClient { } } - private handleErrorState() { - this.state = State.Error; - this.reconnectTryCount += 1; - const delay = - RECONNECT_DELAY_MS[this.reconnectTryCount - 1] || MAX_RECONNECT_DELAY_MS; - log.info('Reconnecting in', delay, 'ms'); - this.reconnectTimer.schedule(delay); - } - private handleReconnect() { if (this.state !== State.Error) { log.error('Unexpected reconnect in', this.state); return; } if (document.visibilityState === 'hidden') { - this.state = State.TimedOut; + this.state = State.ClosedDueToInactivity; } else { this.reconnect(); } -- cgit v1.2.3-70-g09d2