From 3077488a638122c79d457a9f6d05e4f598c0d0c1 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Tue, 8 Dec 2020 12:21:41 -0500 Subject: [PATCH] feat: Remove spawn error event listener Remove spawn error event listener is favour of using stderr stream. --- .../routes/scopes/scope/projects/targets.js | 2 +- ui/desktop/electron-app/src/handlers.js | 10 ++++-- ui/desktop/electron-app/src/spawn-promise.js | 32 +++++++------------ 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets.js b/ui/desktop/app/routes/scopes/scope/projects/targets.js index a42d2bb54..b98e9f416 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets.js @@ -56,7 +56,7 @@ export default class ScopesScopeProjectsTargetsRoute extends Route { connect.then((data) => { this.notify.success(JSON.stringify(data)); }, (e) => { - this.notify.error(e.message, { closeAfter: null }); + this.notify.error(JSON.stringify(e.message), { closeAfter: null }); }); } catch(e) { this.notify.error(e.message, { closeAfter: null }); diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 99b859a65..d747c556c 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -23,7 +23,7 @@ handle('setOrigin', (requestOrigin) => origin = requestOrigin); /** * Detect boundary cli in env path */ -handle('cli', async (event, request) => { +handle('cli', async () => { const cliPath = await lookpath('boundary'); if(!cliPath) throw new Error({ message: 'Cannot find boundary cli.'}) return cliPath; @@ -33,6 +33,12 @@ handle('cli', async (event, request) => { * Establishes a boundary session and returns session details. */ handle('connect', async ({ target_id, token }) => { - const connectCmd = ['connect', `-target-id=${target_id}`, `-token=${token}`, '-format=json']; + const connectCmd = [ + 'connect', + `-target-id=${target_id}`, + `-token=${token}`, + '-format=json', + '--output-json-errors' + ]; return spawnPromise(connectCmd); }); diff --git a/ui/desktop/electron-app/src/spawn-promise.js b/ui/desktop/electron-app/src/spawn-promise.js index 4cae62e46..1fdac9a6e 100644 --- a/ui/desktop/electron-app/src/spawn-promise.js +++ b/ui/desktop/electron-app/src/spawn-promise.js @@ -27,13 +27,13 @@ const resolveOutput = (resolve, outputStream) => { // error from the underlying exec implementation. It's not a very // helpful message for users, so it's recommended to craft nice // POJO representation and throw it or promise->reject it. -// -// TODO: connection errors may now return JSON on stderr, which we -// would attempt to parse here to use a basis for rejection. -const rejectError = (reject, message, code=10001) => { - const error = new Error(`Connect error: ${message}`); - error.code = code; - reject(error); +const rejectError = (reject, message) => { + try { + const errorMessage = JSON.parse(message); + reject(new Error(errorMessage.error)); + } catch (e) { + // Ignore JSON parse errors while stderr buffers. + } }; module.exports = function spawnPromise (command) { @@ -46,20 +46,10 @@ module.exports = function spawnPromise (command) { outputStream += data.toString(); resolveOutput(resolve, outputStream); }); - - // Handle exit of spawned process - childProcess.on('close', (code) => { - if(code === 0) { - resolveOutput(resolve, outputStream); - }else { - rejectError(reject, errorStream); - } + + childProcess.stderr.on('data', (data) => { + errorStream += data.toString(); + rejectError(reject, errorStream); }); - - // Handle error - childProcess.on('error', (err) => rejectError(reject, err)); - - // TODO: Figure out what scenarios this is needed in addition to error event listener - childProcess.stderr.on('data', (data) => errorStream += data.toString()); }); } \ No newline at end of file