Skip to content

Commit 73683a5

Browse files
authored
fix: fix multiple establish connection calls (#1624)
**Summary** This updates our base session class to _wait_ to call `establishConnection` more than once, instead returning the already created promise if we're still waiting on a call to complete. **Testing** - [x] Made sure there was only one call to `contextApi.createSession`
1 parent cf43cce commit 73683a5

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

client/src/connection/session.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export type SessionContextAttributes =
1313

1414
export abstract class Session {
1515
protected _rejectRun: (reason?: unknown) => void | undefined;
16+
protected _connectionPromise: Promise<void> | undefined;
1617

1718
protected _onSessionLogFn: OnLogFn | undefined;
1819
public set onSessionLogFn(value: OnLogFn) {
@@ -25,16 +26,25 @@ export abstract class Session {
2526
}
2627

2728
async setup(silent?: boolean): Promise<void> {
29+
// If we already have a connection promise we're awaiting, lets use that.
30+
// Otherwise, establish a new connection
31+
this._connectionPromise ||= this.establishConnection();
2832
if (silent) {
29-
return await this.establishConnection();
33+
const resolvedData = await this._connectionPromise;
34+
this._connectionPromise = undefined;
35+
return resolvedData;
3036
}
3137

3238
await window.withProgress(
3339
{
3440
location: ProgressLocation.Notification,
3541
title: l10n.t("Connecting to SAS session..."),
3642
},
37-
this.establishConnection,
43+
async () => {
44+
const resolvedData = await this._connectionPromise;
45+
this._connectionPromise = undefined;
46+
return resolvedData;
47+
},
3848
);
3949
}
4050

@@ -57,6 +67,7 @@ export abstract class Session {
5767
this._rejectRun({ message: l10n.t("The SAS session has closed.") });
5868
this._rejectRun = undefined;
5969
}
70+
this._connectionPromise = undefined;
6071
return this._close();
6172
}
6273
protected abstract _close(): Promise<void> | void;
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { expect } from "chai";
2+
import * as sinon from "sinon";
3+
4+
import { RunResult } from "../../src/connection";
5+
import { Session } from "../../src/connection/session";
6+
7+
class MockSession extends Session {
8+
constructor(protected readonly connectionMock: () => void) {
9+
super();
10+
}
11+
protected async establishConnection(): Promise<void> {
12+
return new Promise((resolve) => {
13+
setTimeout(() => {
14+
this.connectionMock();
15+
resolve();
16+
}, 100);
17+
});
18+
}
19+
protected _run(code: string, ...args: any[]): Promise<RunResult> {
20+
throw new Error("Method not implemented.");
21+
}
22+
protected _close(): Promise<void> | void {}
23+
sessionId?(): string | undefined {
24+
return;
25+
}
26+
}
27+
28+
describe("Session test", () => {
29+
it("triggers establish connection only once", async () => {
30+
const mockConnectionFn = sinon.mock();
31+
const mockSession = new MockSession(mockConnectionFn);
32+
const setupPromises: Promise<void>[] = Array(10)
33+
.fill(true)
34+
.map(() => mockSession.setup());
35+
36+
// Wait for everything to wrap up
37+
await Promise.all(setupPromises);
38+
39+
// We called setup 10 times, but we expect to have only called establishConnection
40+
// once.
41+
expect(mockConnectionFn.callCount).to.equal(1);
42+
});
43+
});

0 commit comments

Comments
 (0)