From 403e074249559e5be84f39e9c088b474e852b7e1 Mon Sep 17 00:00:00 2001 From: Kevin Nadro Date: Fri, 30 Aug 2024 05:14:24 -0500 Subject: [PATCH] Nadro/3686/file swapping while executing (#3703) * chore: Implemented a executeAst interrupt to stop processing a KCL program * fix: added a catch since this promise was not being caught * fix: fmt formatting, need to fix some tsc errors next. * fix: fixing tsc errors * fix: cleaning up comment * fix: only rejecting pending modeling commands * fix: adding constant for rejection message, adding rejection in WASM send command * fix: tsc, lint, fmt checks * fix circ dependency --------- Co-authored-by: Kurt Hutten Irev-Dev --- src/lang/KclSingleton.ts | 59 +++++++++++++++++++------------- src/lang/langHelpers.ts | 13 +++++++ src/lang/std/engineConnection.ts | 48 ++++++++++++++++++++++---- src/lib/constants.ts | 5 +++ src/lib/singletons.ts | 1 + 5 files changed, 97 insertions(+), 29 deletions(-) diff --git a/src/lang/KclSingleton.ts b/src/lang/KclSingleton.ts index 6241bd151..7e16c6ed7 100644 --- a/src/lang/KclSingleton.ts +++ b/src/lang/KclSingleton.ts @@ -4,6 +4,7 @@ import { KCLError, kclErrorsToDiagnostics } from './errors' import { uuidv4 } from 'lib/utils' import { EngineCommandManager } from './std/engineConnection' import { err } from 'lib/trap' +import { EXECUTE_AST_INTERRUPT_ERROR_MESSAGE } from 'lib/constants' import { CallExpression, @@ -122,6 +123,7 @@ export class KclManager { get isExecuting() { return this._isExecuting } + set isExecuting(isExecuting) { this._isExecuting = isExecuting // If we have finished executing, but the execute is stale, we should @@ -232,6 +234,12 @@ export class KclManager { async executeAst(args: ExecuteArgs = {}): Promise { if (this.isExecuting) { this.executeIsStale = args + + // The previous execteAst will be rejected and cleaned up. The execution will be marked as stale. + // A new executeAst will start. + this.engineCommandManager.rejectAllModelingCommands( + EXECUTE_AST_INTERRUPT_ERROR_MESSAGE + ) // Exit early if we are already executing. return } @@ -245,35 +253,38 @@ export class KclManager { // Make sure we clear before starting again. End session will do this. this.engineCommandManager?.endSession() await this.ensureWasmInit() - const { logs, errors, programMemory } = await executeAst({ + const { logs, errors, programMemory, isInterrupted } = await executeAst({ ast, engineCommandManager: this.engineCommandManager, }) - this.lints = await lintAst({ ast: ast }) + // Program was not interrupted, setup the scene + // Do not send send scene commands if the program was interrupted, go to clean up + if (!isInterrupted) { + this.lints = await lintAst({ ast: ast }) - sceneInfra.modelingSend({ type: 'code edit during sketch' }) - defaultSelectionFilter(programMemory, this.engineCommandManager) - await this.engineCommandManager.waitForAllCommands() + sceneInfra.modelingSend({ type: 'code edit during sketch' }) + defaultSelectionFilter(programMemory, this.engineCommandManager) - if (args.zoomToFit) { - let zoomObjectId: string | undefined = '' - if (args.zoomOnRangeAndType) { - zoomObjectId = this.engineCommandManager?.mapRangeToObjectId( - args.zoomOnRangeAndType.range, - args.zoomOnRangeAndType.type - ) + if (args.zoomToFit) { + let zoomObjectId: string | undefined = '' + if (args.zoomOnRangeAndType) { + zoomObjectId = this.engineCommandManager?.mapRangeToObjectId( + args.zoomOnRangeAndType.range, + args.zoomOnRangeAndType.type + ) + } + + await this.engineCommandManager.sendSceneCommand({ + type: 'modeling_cmd_req', + cmd_id: uuidv4(), + cmd: { + type: 'zoom_to_fit', + object_ids: zoomObjectId ? [zoomObjectId] : [], // leave empty to zoom to all objects + padding: 0.1, // padding around the objects + }, + }) } - - await this.engineCommandManager.sendSceneCommand({ - type: 'modeling_cmd_req', - cmd_id: uuidv4(), - cmd: { - type: 'zoom_to_fit', - object_ids: zoomObjectId ? [zoomObjectId] : [], // leave empty to zoom to all objects - padding: 0.1, // padding around the objects - }, - }) } this.isExecuting = false @@ -284,7 +295,8 @@ export class KclManager { return } this.logs = logs - this.addKclErrors(errors) + // Do not add the errors since the program was interrupted and the error is not a real KCL error + this.addKclErrors(isInterrupted ? [] : errors) this.programMemory = programMemory this.ast = { ...ast } this._executeCallback() @@ -292,6 +304,7 @@ export class KclManager { type: 'execution-done', data: null, }) + this._cancelTokens.delete(currentExecutionId) } // NOTE: this always updates the code state and editor. diff --git a/src/lang/langHelpers.ts b/src/lang/langHelpers.ts index 17ba2a8e2..f2b69822f 100644 --- a/src/lang/langHelpers.ts +++ b/src/lang/langHelpers.ts @@ -54,10 +54,12 @@ export async function executeAst({ engineCommandManager: EngineCommandManager useFakeExecutor?: boolean programMemoryOverride?: ProgramMemory + isInterrupted?: boolean }): Promise<{ logs: string[] errors: KCLError[] programMemory: ProgramMemory + isInterrupted: boolean }> { try { if (!useFakeExecutor) { @@ -73,13 +75,23 @@ export async function executeAst({ logs: [], errors: [], programMemory, + isInterrupted: false, } } catch (e: any) { + let isInterrupted = false if (e instanceof KCLError) { + // Detect if it is a force interrupt error which is not a KCL processing error. + if ( + e.msg === + 'Failed to wait for promise from engine: JsValue("Force interrupt, executionIsStale, new AST requested")' + ) { + isInterrupted = true + } return { errors: [e], logs: [], programMemory: ProgramMemory.empty(), + isInterrupted, } } else { console.log(e) @@ -87,6 +99,7 @@ export async function executeAst({ logs: [e], errors: [], programMemory: ProgramMemory.empty(), + isInterrupted, } } } diff --git a/src/lang/std/engineConnection.ts b/src/lang/std/engineConnection.ts index d426863e6..169b198d4 100644 --- a/src/lang/std/engineConnection.ts +++ b/src/lang/std/engineConnection.ts @@ -16,6 +16,8 @@ import { useModelingContext } from 'hooks/useModelingContext' import { exportMake } from 'lib/exportMake' import toast from 'react-hot-toast' import { SettingsViaQueryString } from 'lib/settings/settingsTypes' +import { EXECUTE_AST_INTERRUPT_ERROR_MESSAGE } from 'lib/constants' +import { KclManager } from 'lang/KclSingleton' // TODO(paultag): This ought to be tweakable. const pingIntervalMs = 5_000 @@ -1279,6 +1281,7 @@ interface PendingMessage { resolve: (data: [Models['WebSocketResponse_type']]) => void reject: (reason: string) => void promise: Promise<[Models['WebSocketResponse_type']]> + isSceneCommand: boolean } export class EngineCommandManager extends EventTarget { /** @@ -1379,6 +1382,7 @@ export class EngineCommandManager extends EventTarget { }: CustomEvent) => {} modelingSend: ReturnType['send'] = (() => {}) as any + kclManager: null | KclManager = null set exportIntent(intent: ExportIntent | null) { this._exportIntent = intent @@ -1932,11 +1936,21 @@ export class EngineCommandManager extends EventTarget { ;(cmd as any).sequence = this.outSequence++ } // since it's not mouse drag or highlighting send over TCP and keep track of the command - return this.sendCommand(command.cmd_id, { - command, - idToRangeMap: {}, - range: [0, 0], - }).then(([a]) => a) + return this.sendCommand( + command.cmd_id, + { + command, + idToRangeMap: {}, + range: [0, 0], + }, + true // isSceneCommand + ) + .then(([a]) => a) + .catch((e) => { + // TODO: Previously was never caught, we are not rejecting these pendingCommands but this needs to be handled at some point. + /*noop*/ + return null + }) } /** * A wrapper around the sendCommand where all inputs are JSON strings @@ -1963,6 +1977,12 @@ export class EngineCommandManager extends EventTarget { const idToRangeMap: { [key: string]: SourceRange } = JSON.parse(idToRangeStr) + // Current executeAst is stale, going to interrupt, a new executeAst will trigger + // Used in conjunction with rejectAllModelingCommands + if (this?.kclManager?.executeIsStale) { + return Promise.reject(EXECUTE_AST_INTERRUPT_ERROR_MESSAGE) + } + const resp = await this.sendCommand(id, { command, range, @@ -1980,7 +2000,8 @@ export class EngineCommandManager extends EventTarget { command: PendingMessage['command'] range: PendingMessage['range'] idToRangeMap: PendingMessage['idToRangeMap'] - } + }, + isSceneCommand = false ): Promise<[Models['WebSocketResponse_type']]> { const { promise, resolve, reject } = promiseFactory() this.pendingCommands[id] = { @@ -1990,7 +2011,9 @@ export class EngineCommandManager extends EventTarget { command: message.command, range: message.range, idToRangeMap: message.idToRangeMap, + isSceneCommand, } + if (message.command.type === 'modeling_cmd_req') { this.orderedCommands.push({ command: message.command, @@ -2037,6 +2060,19 @@ export class EngineCommandManager extends EventTarget { this.deferredArtifactPopulated(null) } } + + /** + * Reject all of the modeling pendingCommands created from sendModelingCommandFromWasm + * This interrupts the runtime of executeAst. Stops the AST processing and stops sending commands + * to the engine + */ + rejectAllModelingCommands(rejectionMessage: string) { + Object.values(this.pendingCommands).forEach( + ({ reject, isSceneCommand }) => + !isSceneCommand && reject(rejectionMessage) + ) + } + async initPlanes() { if (this.planesInitialized()) return const planes = await this.makeDefaultPlanes() diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 3c7f2b7f3..cacd8d312 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -67,3 +67,8 @@ export const COOKIE_NAME = '__Secure-next-auth.session-token' /** localStorage key to determine if we're in Playwright tests */ export const PLAYWRIGHT_KEY = 'playwright' + +/** Custom error message to match when rejectAllModelCommands is called + * allows us to match if the execution of executeAst was interrupted */ +export const EXECUTE_AST_INTERRUPT_ERROR_MESSAGE = + 'Force interrupt, executionIsStale, new AST requested' diff --git a/src/lib/singletons.ts b/src/lib/singletons.ts index 17e5cf25e..2e6df784e 100644 --- a/src/lib/singletons.ts +++ b/src/lib/singletons.ts @@ -17,6 +17,7 @@ window.tearDown = engineCommandManager.tearDown // This needs to be after codeManager is created. export const kclManager = new KclManager(engineCommandManager) kclManager.isFirstRender = true +engineCommandManager.kclManager = kclManager engineCommandManager.getAstCb = () => kclManager.ast