From 811cc2f81046a01f67e5f4c02fcded93a6e6588e Mon Sep 17 00:00:00 2001 From: seem Date: Thu, 28 Nov 2024 13:53:31 +0200 Subject: [PATCH] fix uncaught error --- .../runtimeSession/common/runtimeSession.ts | 27 +++++++--------- .../test/common/runtimeSession.test.ts | 31 ++++++++++++++++++- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts b/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts index 2af31f2a661..ec9d2016eab 100644 --- a/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts +++ b/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as nls from 'vs/nls'; -import { DeferredPromise } from 'vs/base/common/async'; +import { DeferredPromise, disposableTimeout } from 'vs/base/common/async'; import { Emitter } from 'vs/base/common/event'; import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; @@ -377,33 +377,30 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession // We wait for `onDidEndSession()` rather than `RuntimeState.Exited`, because the former // generates some Console output that must finish before starting up a new runtime: - let disposable: IDisposable | undefined; - const promise = new Promise(resolve => { - disposable = sessionDisposables.add(session.onDidEndSession((exit) => { + const disposables = sessionDisposables.add(new DisposableStore()); + const promise = new Promise((resolve, reject) => { + disposables.add(session.onDidEndSession((exit) => { + disposables.dispose(); resolve(); - disposable?.dispose(); })); - }); - - const timeout = new Promise((_, reject) => { - setTimeout(() => { - disposable?.dispose(); + disposables.add(disposableTimeout(() => { + disposables.dispose(); reject(new Error(`Timed out waiting for runtime ` + `${formatLanguageRuntimeSession(session)} to finish exiting.`)); - }, 5000); + }, 5000)); }); // Ask the runtime to shut down. try { await session.shutdown(exitReason); - } catch (err) { - disposable?.dispose(); - throw err; + } catch (error) { + disposables.dispose(); + throw error; } // Wait for the runtime onDidEndSession to resolve, or for the timeout to expire // (whichever comes first) - await Promise.race([promise, timeout]); + await promise; } /** diff --git a/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts b/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts index 482473a2077..85aa3b003ee 100644 --- a/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts +++ b/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts @@ -12,7 +12,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/workspaceTrust'; -import { formatLanguageRuntimeMetadata, ILanguageRuntimeMetadata, ILanguageRuntimeService, LanguageRuntimeSessionMode, RuntimeExitReason, RuntimeState } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService'; +import { formatLanguageRuntimeMetadata, formatLanguageRuntimeSession, ILanguageRuntimeMetadata, ILanguageRuntimeService, LanguageRuntimeSessionMode, RuntimeExitReason, RuntimeState } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService'; import { ILanguageRuntimeSession, IRuntimeSessionMetadata, IRuntimeSessionService, IRuntimeSessionWillStartEvent } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService'; import { TestLanguageRuntimeSession, waitForRuntimeState } from 'vs/workbench/services/runtimeSession/test/common/testLanguageRuntimeSession'; import { createRuntimeServices, createTestLanguageRuntimeMetadata, startTestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService'; @@ -653,6 +653,35 @@ suite('Positron - RuntimeSessionService', () => { assert.equal(runtimeSessionService.foregroundSession, session1); }); + test(`select console to another runtime and first session never fires onDidEndSession`, async () => { + const session = await startConsole(); + await waitForRuntimeState(session, RuntimeState.Ready); + + // Stub onDidEndSession to never fire, causing the shutdown to time out. + sinon.stub(session, 'onDidEndSession').returns({ dispose: () => { } }); + + // Use a fake timer to avoid actually having to wait for the timeout. + const clock = sinon.useFakeTimers(); + const promise = assert.rejects(selectRuntime(anotherRuntime), new Error(`Timed out waiting for runtime ` + + `${formatLanguageRuntimeSession(session)} to finish exiting.`)); + await clock.tickAsync(10_000); + await promise; + }); + + test(`select console to another runtime encounters session.shutdown() error`, async () => { + const session = await startConsole(); + + // Stub session.shutdown() to throw an error. + const error = new Error('Session failed to shut down'); + sinon.stub(session, 'shutdown').rejects(error); + + // We also want to ensure that the timeout is not hit in this case but don't want to + // actually wait, so we use a fake timer. + const clock = sinon.useFakeTimers(); + await assert.rejects(selectRuntime(anotherRuntime), error); + await clock.tickAsync(10_000); + }); + function restartSession(sessionId: string) { return runtimeSessionService.restartSession(sessionId, startReason); }