From e3eedc83d1648e8775d2dc2facee6e6edc7e4916 Mon Sep 17 00:00:00 2001 From: seem Date: Fri, 15 Nov 2024 16:50:02 +0200 Subject: [PATCH] test encountering createSession/restoreSession error; catch uncaught exceptions --- .../runtimeSession/common/runtimeSession.ts | 18 ++++++++++++------ .../test/common/runtimeSession.test.ts | 8 ++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts b/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts index 432585249ab..f3e9252f64c 100644 --- a/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts +++ b/src/vs/workbench/services/runtimeSession/common/runtimeSession.ts @@ -479,6 +479,10 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession sessionMetadata.sessionMode, runtimeMetadata.runtimeId, sessionMetadata.notebookUri); this._startingSessionsBySessionMapKey.set(sessionMapKey, startPromise); + // It's possible that startPromise is never awaited, so we log any errors here + // at the debug level since we still expect the error to be handled/logged elsewhere. + startPromise.p.catch((err) => this._logService.debug(`Error starting session: ${err}`)); + // Add the runtime to the starting runtimes. if (sessionMetadata.sessionMode === LanguageRuntimeSessionMode.Console) { this._startingConsolesByLanguageId.set(runtimeMetadata.languageId, runtimeMetadata); @@ -517,8 +521,6 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession } catch (err) { startPromise.error(err); } - - startPromise.p.catch((err) => this._logService.debug(`Error starting session: ${err}`)); } /** @@ -725,6 +727,10 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession const sessionMapKey = getSessionMapKey(sessionMode, metadata.runtimeId, notebookUri); this._startingSessionsBySessionMapKey.set(sessionMapKey, startPromise); + // It's possible that startPromise is never awaited, so we log any errors here + // at the debug level since we still expect the error to be handled/logged elsewhere. + startPromise.p.catch(err => this._logService.debug(`Error starting runtime session: ${err}`)); + // Check to see if the runtime has already been registered with the // language runtime service. const languageRuntime = @@ -818,6 +824,10 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession if (!startPromise || startPromise.isSettled) { startPromise = new DeferredPromise(); this._startingSessionsBySessionMapKey.set(sessionMapKey, startPromise); + + // It's possible that startPromise is never awaited, so we log any errors here + // at the debug level since we still expect the error to be handled/logged elsewhere. + startPromise.p.catch(err => this._logService.debug(`Error starting runtime session: ${err}`)); } const sessionManager = await this.getManagerForRuntime(runtimeMetadata); @@ -858,10 +868,6 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession startPromise.error(err); } - // It's possible that startPromise is never awaited, so we log any errors here - // at the debug level since we still expect the error to be handled/logged elsewhere. - startPromise.p.catch(err => this._logService.debug(`Error starting runtime session: ${err}`)); - return sessionId; } 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 6f014646397..65ca7fd854c 100644 --- a/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts +++ b/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts @@ -341,6 +341,14 @@ suite('Positron - RuntimeSessionService', () => { }); } + const createOrRestoreMethod = action === 'restore' ? 'restoreSession' : 'createSession'; + test(`${action} ${mode} encounters ${createOrRestoreMethod}() error`, async () => { + const error = new Error('Failed to create session'); + sinon.stub(manager, createOrRestoreMethod).rejects(error); + + await assert.rejects(start(), error); + }); + test(`${action} ${mode} fires onDidFailStartRuntime if session.start() errors`, async () => { // Listen to the onWillStartSession event and stub session.start() to throw an error. const willStartSession = sinon.spy((e: IRuntimeSessionWillStartEvent) => {