From 2be18bcf6f9a5f335d88ee684d1680c093f62b22 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Tue, 11 May 2021 18:09:38 -0400 Subject: [PATCH] chore: Cleanup --- .../routes/scopes/scope/projects/sessions.js | 6 +--- .../scope/projects/targets/target/sessions.js | 6 +--- ui/desktop/electron-app/src/handlers.js | 5 +-- ui/desktop/electron-app/src/session/index.js | 31 +++++++++++++++---- .../electron-app/src/session/manager.js | 16 ++++++++-- .../acceptance/projects/sessions-test.js | 17 ++++++++++ .../projects/targets/sessions-test.js | 12 +++++++ 7 files changed, 71 insertions(+), 22 deletions(-) diff --git a/ui/desktop/app/routes/scopes/scope/projects/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/sessions.js index 2715a7aa3..908636f1a 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/sessions.js @@ -84,10 +84,6 @@ export default class ScopesScopeProjectsSessionsRoute extends Route { @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); - try { - this.ipc.invoke('cancel', { session_id: session.id }); - } catch (e) { - //Ignore errors - } + await this.ipc.invoke('cancel', { session_id: session.id }); } } diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js index 2a849f3b9..2263aa448 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js @@ -19,10 +19,6 @@ export default class ScopesScopeProjectsTargetsTargetSessionsRoute extends Route @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); - try { - this.ipc.invoke('cancel', { session_id: session.id }); - } catch (e) { - //Ignore errors - } + await this.ipc.invoke('cancel', { session_id: session.id }); } } diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 7127c1f12..3aee556f9 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -63,7 +63,4 @@ handle('connect', ({ target_id, token, host_id }) => /** * Cancel an established boundary session spawned process. */ -handle('cancel', ({ session_id }) => - // spawnSession.cancelSession(session_id); - sessionManager.stop(session_id) -); +handle('cancel', ({ session_id }) => sessionManager.stopById(session_id)); diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js index 3d5563f1d..cfc760ba7 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/session/index.js @@ -1,4 +1,4 @@ -const { spawnAsyncJSONPromise } = require('../spawn-promise'); +const { spawnAsyncJSONPromise } = require('../spawn-promise.js'); // FIXME create structure 'util' folder for spawn-promise @@ -21,6 +21,7 @@ class Session { #addr; #token; #hostId; + #process; #targetId; #proxyDetails; @@ -39,11 +40,19 @@ class Session { this.#hostId = hostId; } + /** + * Session id + * @return {string} + */ + get id() { + return this.#id; + } + /** * @return {boolean} */ get isActive() { - return Boolean(this.#pid); + return this.#process.connected; } /** @@ -55,8 +64,6 @@ class Session { return this.#proxyDetails; } - // Spawn process - // Track spawned process this.pid = pid; /** * Using cli, initialize a session to a target. * Tracks local proxy details if successful. @@ -64,9 +71,21 @@ class Session { start() { const command = this.cliCommand(); return spawnAsyncJSONPromise(command).then((spawnedSession) => { - this.#id = this.#proxyDetails.session_id; - this.#pid = spawnedSession.childProcess.pid; + this.#process = spawnedSession.childProcess; this.#proxyDetails = spawnedSession.response; + this.#process = spawnedSession.childProcess; + this.#id = this.#proxyDetails.session_id; + }); + } + + /** + * Stop proxy used by session. + */ + stop() { + return new Promise((resolve, reject) => { + this.#process.on('close', () => resolve()); + this.#process.on('error', (e) => reject(e)); + this.#process.kill(); }); } diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/session/manager.js index 60be77204..840ee76f3 100644 --- a/ui/desktop/electron-app/src/session/manager.js +++ b/ui/desktop/electron-app/src/session/manager.js @@ -16,7 +16,7 @@ class SessionManager { * @param {string} token * @param {string} host_id */ - start(addr, target_id, token, host_id) { + async start(addr, target_id, token, host_id) { const session = new Session(addr, target_id, token, host_id); return session.start().then(() => { this.#sessions.push(session); @@ -24,7 +24,19 @@ class SessionManager { }); } - stopById() { + /** + * Stop a session and remove it from tracked sessions. + * @param {string} session_id + */ + async stopById(session_id) { + const index = this.#sessions.findIndex( + (session) => session.id === session_id + ); + if (index === -1) return; + const session = this.#sessions[index]; + return session.stop().then(() => { + this.#sessions.splice(index, 1); + }); } stopAll() { diff --git a/ui/desktop/tests/acceptance/projects/sessions-test.js b/ui/desktop/tests/acceptance/projects/sessions-test.js index eb3e0bab1..012db14cb 100644 --- a/ui/desktop/tests/acceptance/projects/sessions-test.js +++ b/ui/desktop/tests/acceptance/projects/sessions-test.js @@ -19,6 +19,7 @@ import { authenticateSession, invalidateSession, } from 'ember-simple-auth/test-support'; +import sinon from 'sinon'; module('Acceptance | projects | sessions', function (hooks) { setupApplicationTest(hooks); @@ -43,6 +44,7 @@ module('Acceptance | projects | sessions', function (hooks) { const stubs = { global: null, org: null, + ipcService: null, }; const urls = { @@ -144,6 +146,9 @@ module('Acceptance | projects | sessions', function (hooks) { window.addEventListener('message', messageHandler); setDefaultOrigin(this); + + const ipcService = this.owner.lookup('service:ipc'); + stubs.ipcService = sinon.stub(ipcService, 'invoke'); }); hooks.afterEach(function () { @@ -240,6 +245,7 @@ module('Acceptance | projects | sessions', function (hooks) { test('cancelling a session', async function (assert) { assert.expect(1); + stubs.ipcService.withArgs('cancel'); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); @@ -258,4 +264,15 @@ module('Acceptance | projects | sessions', function (hooks) { }, 750); await visit(urls.sessions); }); + + test('cancelling a session with ipc error shows notification', async function (assert) { + assert.expect(1); + stubs.ipcService.withArgs('cancel').throws(); + later(async () => { + run.cancelTimers(); + await click('tbody tr:first-child td:last-child button'); + assert.ok(find('[role="alert"].is-error')); + }, 750); + await visit(urls.sessions); + }); }); diff --git a/ui/desktop/tests/acceptance/projects/targets/sessions-test.js b/ui/desktop/tests/acceptance/projects/targets/sessions-test.js index a28682950..9fde4b210 100644 --- a/ui/desktop/tests/acceptance/projects/targets/sessions-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/sessions-test.js @@ -248,6 +248,7 @@ module('Acceptance | projects | targets | sessions', function (hooks) { test('cancelling a session', async function (assert) { assert.expect(2); const sessionsCount = this.server.schema.sessions.all().models.length; + stubs.ipcService.withArgs('cancel'); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); @@ -268,6 +269,17 @@ module('Acceptance | projects | targets | sessions', function (hooks) { await visit(urls.sessions); }); + test('cancelling a session with ipc error shows notification', async function (assert) { + assert.expect(1); + stubs.ipcService.withArgs('cancel').throws(); + later(async () => { + run.cancelTimers(); + await click('tbody tr:first-child td:last-child button'); + assert.ok(find('[role="alert"].is-error')); + }, 750); + await visit(urls.sessions); + }); + test('connecting to a target', async function (assert) { assert.expect(3); stubs.ipcService.withArgs('cliExists').returns(true);