From cc24e5e03eee430a50c833aa1c21d76c54fc9205 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Tue, 8 Dec 2020 10:11:24 -0500 Subject: [PATCH] feat: Add node spawn promise wrapper --- .../routes/scopes/scope/projects/targets.js | 9 ++- ui/desktop/app/services/ipc.js | 2 +- ui/desktop/electron-app/src/handlers.js | 56 ++++------------ ui/desktop/electron-app/src/spawn-promise.js | 65 +++++++++++++++++++ yarn.lock | 27 ++------ 5 files changed, 91 insertions(+), 68 deletions(-) create mode 100644 ui/desktop/electron-app/src/spawn-promise.js diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets.js b/ui/desktop/app/routes/scopes/scope/projects/targets.js index 199a09878..a42d2bb54 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets.js @@ -47,10 +47,17 @@ export default class ScopesScopeProjectsTargetsRoute extends Route { @action async connect(model) { try { - await this.ipc.invoke('connect', { + await this.ipc.invoke('cli'); + const connect = this.ipc.invoke('connect', { target_id: model.target.id, token: this.session.data.authenticated.token, }); + + connect.then((data) => { + this.notify.success(JSON.stringify(data)); + }, (e) => { + this.notify.error(e.message, { closeAfter: null }); + }); } catch(e) { this.notify.error(e.message, { closeAfter: null }); } diff --git a/ui/desktop/app/services/ipc.js b/ui/desktop/app/services/ipc.js index d1295d052..8be63359f 100644 --- a/ui/desktop/app/services/ipc.js +++ b/ui/desktop/app/services/ipc.js @@ -17,7 +17,7 @@ import { Promise } from 'rsvp'; * knowledge of the main process or the Electron context. In fact a main * process and Electron context are not even required. * - * In an Electron context, the `preload.js` script sets up a `window.postMessge` + * In an Electron context, the `preload.js` script sets up a `window.postMessage` * listener which acts as proxy layer between the main and renderer processes. * It waits for incoming requests from the renderer and forwards them to main. * Responses from the main process are forwarded back via a response port diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 8803ddd6e..99b859a65 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -1,6 +1,6 @@ const handle = require('./ipc-handler'); const { lookpath } = require('lookpath'); -const { exec } = require('child_process'); +const spawnPromise = require('./spawn-promise'); let origin = ''; @@ -20,51 +20,19 @@ handle('getOrigin', () => origin); */ handle('setOrigin', (requestOrigin) => origin = requestOrigin); +/** + * Detect boundary cli in env path + */ +handle('cli', async (event, request) => { + const cliPath = await lookpath('boundary'); + if(!cliPath) throw new Error({ message: 'Cannot find boundary cli.'}) + return cliPath; +}); + /** * Establishes a boundary session and returns session details. - * TODO: Return session details. */ handle('connect', async ({ target_id, token }) => { - const cliAvailable = await lookpath('boundary'); - if(!cliAvailable) { throw new Error('CLI unavailable.'); } - - const connectCmd = `boundary connect -target-id=${target_id} -token=${token}`; - - // TODO: this isn't the *exact* promise, because apparently exec doesn't - // call the success callback until the process exits. We need a way to - // resolve the promise if the process doesn't exit AND we receive valid - // JSON on stdout, since this indicates the connection was successful. - // - // TODO: if there is an existing promiserified exec tool, we might consider - // using it to help make this cleaner. - - // You can throw exceptions, or allow them to occur, and this is supported. - // Exceptions thrown in this way will be returned to the UI as an - // IPC rejection. However in most cases, you probably want to return a - // specific exception object of your own design, since Error instances - // originating from the underlying system will be noisey and not very helpful - // to users. - // - //throw new Error('Random error! Should still work!'); - //throw { message: 'Random POJO exception! Should still work!' }; - - return new Promise((resolve, reject) => { - exec(connectCmd, (error, stdout, stderr) => { - if (error) { - // For example, you could just reject(error) here, but this is the raw - // 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 { code } = error; - return reject({ - code, - message: `OH NOSE! Command failed with code ${code}` - }); - } - return resolve(stdout); - }); - }); + const connectCmd = ['connect', `-target-id=${target_id}`, `-token=${token}`, '-format=json']; + return spawnPromise(connectCmd); }); diff --git a/ui/desktop/electron-app/src/spawn-promise.js b/ui/desktop/electron-app/src/spawn-promise.js new file mode 100644 index 000000000..4cae62e46 --- /dev/null +++ b/ui/desktop/electron-app/src/spawn-promise.js @@ -0,0 +1,65 @@ +const { spawn } = require('child_process'); + +/** + * Attempt to parse output string to JSON. + * When JSON is successfully parsed, consider stdout fully read. +*/ +// TODO: Potential use for BufferList? +const resolveOutput = (resolve, outputStream) => { + try { + resolve(JSON.parse(outputStream)); + } catch (e) { + // Ignore JSON parse errors while stdout buffers. + } +}; + +// You can throw exceptions, or allow them to occur, and this is supported. +// Exceptions thrown in this way will be returned to the UI as an +// IPC rejection. However in most cases, you probably want to return a +// specific exception object of your own design, since Error instances +// originating from the underlying system will be noisey and not very helpful +// to users. +// +//throw new Error('Random error! Should still work!'); +//throw { message: 'Random POJO exception! Should still work!' }; + +// For example, you could just reject(error) here, but this is the raw +// 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); +}; + +module.exports = function spawnPromise (command) { + return new Promise((resolve, reject) => { + const childProcess = spawn('boundary', command); + let outputStream = ''; + let errorStream = ''; + + childProcess.stdout.on('data', (data) => { + 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); + } + }); + + // 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 diff --git a/yarn.lock b/yarn.lock index 6e151364c..1414b408a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7507,7 +7507,7 @@ debug@~4.1.0: dependencies: ms "^2.1.1" -debuglog@*, debuglog@^1.0.1: +debuglog@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/debuglog/-/debuglog-1.0.1.tgz#aa24ffb9ac3df9a2351837cfb2d279360cd78492" integrity sha1-qiT/uaw9+aI1GDfPstJ5NgzXhJI= @@ -12204,7 +12204,7 @@ import-lazy@^4.0.0: resolved "https://registry.yarnpkg.com/import-lazy/-/import-lazy-4.0.0.tgz#e8eb627483a0a43da3c03f3e35548be5cb0cc153" integrity sha512-rKtvo6a868b5Hu3heneU+L4yEQ4jYKLtjpnPeUdK7h0yzXGmyBTypknlkCvHFBqfX9YlorEiMM6Dnq/5atfHkw== -imurmurhash@*, imurmurhash@^0.1.4: +imurmurhash@^0.1.4: version "0.1.4" resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea" integrity sha1-khi5srkoojixPcT7a21XbyMUU+o= @@ -13911,11 +13911,6 @@ lodash._baseflatten@^3.0.0: lodash.isarguments "^3.0.0" lodash.isarray "^3.0.0" -lodash._baseindexof@*: - version "3.1.0" - resolved "https://registry.yarnpkg.com/lodash._baseindexof/-/lodash._baseindexof-3.1.0.tgz#fe52b53a1c6761e42618d654e4a25789ed61822c" - integrity sha1-/lK1OhxnYeQmGNZU5KJXie1hgiw= - lodash._baseuniq@~4.6.0: version "4.6.0" resolved "https://registry.yarnpkg.com/lodash._baseuniq/-/lodash._baseuniq-4.6.0.tgz#0ebb44e456814af7905c6212fa2c9b2d51b841e8" @@ -13924,16 +13919,11 @@ lodash._baseuniq@~4.6.0: lodash._createset "~4.0.0" lodash._root "~3.0.0" -lodash._bindcallback@*, lodash._bindcallback@^3.0.0: +lodash._bindcallback@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/lodash._bindcallback/-/lodash._bindcallback-3.0.1.tgz#e531c27644cf8b57a99e17ed95b35c748789392e" integrity sha1-5THCdkTPi1epnhftlbNcdIeJOS4= -lodash._cacheindexof@*: - version "3.0.2" - resolved "https://registry.yarnpkg.com/lodash._cacheindexof/-/lodash._cacheindexof-3.0.2.tgz#3dc69ac82498d2ee5e3ce56091bafd2adc7bde92" - integrity sha1-PcaayCSY0u5ePOVgkbr9Ktx73pI= - lodash._createassigner@^3.0.0: version "3.1.1" resolved "https://registry.yarnpkg.com/lodash._createassigner/-/lodash._createassigner-3.1.1.tgz#838a5bae2fdaca63ac22dee8e19fa4e6d6970b11" @@ -13943,19 +13933,12 @@ lodash._createassigner@^3.0.0: lodash._isiterateecall "^3.0.0" lodash.restparam "^3.0.0" -lodash._createcache@*: - version "3.1.2" - resolved "https://registry.yarnpkg.com/lodash._createcache/-/lodash._createcache-3.1.2.tgz#56d6a064017625e79ebca6b8018e17440bdcf093" - integrity sha1-VtagZAF2JeeevKa4AY4XRAvc8JM= - dependencies: - lodash._getnative "^3.0.0" - lodash._createset@~4.0.0: version "4.0.3" resolved "https://registry.yarnpkg.com/lodash._createset/-/lodash._createset-4.0.3.tgz#0f4659fbb09d75194fa9e2b88a6644d363c9fe26" integrity sha1-D0ZZ+7CddRlPqeK4imZE02PJ/iY= -lodash._getnative@*, lodash._getnative@^3.0.0: +lodash._getnative@^3.0.0: version "3.9.1" resolved "https://registry.yarnpkg.com/lodash._getnative/-/lodash._getnative-3.9.1.tgz#570bc7dede46d61cdcde687d65d3eecbaa3aaff5" integrity sha1-VwvH3t5G1hzc3mh9ZdPuy6o6r/U= @@ -14168,7 +14151,7 @@ lodash.pick@^4.4.0: resolved "https://registry.yarnpkg.com/lodash.pick/-/lodash.pick-4.4.0.tgz#52f05610fff9ded422611441ed1fc123a03001b3" integrity sha1-UvBWEP/53tQiYRRB7R/BI6AwAbM= -lodash.restparam@*, lodash.restparam@^3.0.0: +lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU=