[cordova-common] branch master updated: Catch leaked exceptions in superspawn and convert them to rejections

[ra...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-common.git


The following commit(s) were added to refs/heads/master by this push:
     new 469e7b4  Catch leaked exceptions in superspawn and convert them to rejections
469e7b4 is described below

commit 469e7b4a49a1a39c7fc22aa206730b9d58613467
Author: Raphael von der Grün <ra...@gmail.com>
AuthorDate: Mon Apr 1 16:47:27 2019 +0200

    Catch leaked exceptions in superspawn and convert them to rejections
    
    At least until Node.js 8, child_process.spawn will throw exceptions
    instead of emitting error events in certain cases (like EACCES), Thus we
    have to wrap the execution in try/catch to convert them into rejections.
---
 spec/superspawn.spec.js | 19 +++++++++++++++++++
 src/superspawn.js       | 16 +++++++++++++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/spec/superspawn.spec.js b/spec/superspawn.spec.js
index 95056dc..9c05129 100644
--- a/spec/superspawn.spec.js
+++ b/spec/superspawn.spec.js
@@ -89,6 +89,25 @@ describe('spawn method', function () {
             });
     });
 
+    it('Test 005 : should not throw but reject', () => {
+        if (process.platform === 'win32') {
+            pending('Test should not run on Windows');
+        }
+
+        // Our non-executable (as in no execute permission) script
+        const TEST_SCRIPT = path.join(__dirname, 'fixtures/echo-args.cmd');
+
+        let promise;
+        expect(() => { promise = superspawn.spawn(TEST_SCRIPT, []); }).not.toThrow();
+
+        return Promise.resolve(promise).then(_ => {
+            fail('Expected promise to be rejected');
+        }, err => {
+            expect(err).toEqual(jasmine.any(Error));
+            expect(err.code).toBe('EACCES');
+        });
+    });
+
     describe('operation on windows', () => {
         const TEST_SCRIPT = path.join(__dirname, 'fixtures/echo-args.cmd');
         const TEST_ARGS = [ 'install', 'foo@^1.2.3', 'c o r d o v a' ];
diff --git a/src/superspawn.js b/src/superspawn.js
index 22daf88..2e58b2e 100644
--- a/src/superspawn.js
+++ b/src/superspawn.js
@@ -87,7 +87,15 @@ exports.spawn = function (cmd, args, opts) {
 
     events.emit(opts.printCommand ? 'log' : 'verbose', 'Running command: ' + cmd + ' ' + args.join(' '));
 
-    var child = crossSpawn.spawn(cmd, args, spawnOpts);
+    // At least until Node.js 8, child_process.spawn will throw exceptions
+    // instead of emitting error events in certain cases (like EACCES), Thus we
+    // have to wrap the execution in try/catch to convert them into rejections.
+    try {
+        var child = crossSpawn.spawn(cmd, args, spawnOpts);
+    } catch (e) {
+        whenDone(e);
+        return d.promise;
+    }
     var capturedOut = '';
     var capturedErr = '';
 
@@ -110,8 +118,10 @@ exports.spawn = function (cmd, args, opts) {
     child.on('close', whenDone);
     child.on('error', whenDone);
     function whenDone (arg) {
-        child.removeListener('close', whenDone);
-        child.removeListener('error', whenDone);
+        if (child) {
+            child.removeListener('close', whenDone);
+            child.removeListener('error', whenDone);
+        }
         var code = typeof arg === 'number' ? arg : arg && arg.code;
 
         events.emit('verbose', 'Command finished with error code ' + code + ': ' + cmd + ' ' + args);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org