Skip to content

Commit

Permalink
test encountering createSession/restoreSession error; catch uncaught …
Browse files Browse the repository at this point in the history
…exceptions
  • Loading branch information
seeM committed Nov 19, 2024
1 parent 186c99a commit e3eedc8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
18 changes: 12 additions & 6 deletions src/vs/workbench/services/runtimeSession/common/runtimeSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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}`));
}

/**
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -818,6 +824,10 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
if (!startPromise || startPromise.isSettled) {
startPromise = new DeferredPromise<string>();
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);
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit e3eedc8

Please sign in to comment.