Merge pull request #214 from extrabacon/fix_stderr_being_emitted_in_binary_mode

Fix stderr being emitted in binary mode & other misc changes

Author: Almenon

README.md

@@ -281,9 +281,9 @@ Parses incoming logs from the Python script written via stderr and emits `stderr
 
 Closes the stdin stream, allowing the Python script to finish and exit. The optional callback is invoked when the process is terminated.
 
-#### `.terminate(signal)`
+#### `.kill(signal)`
 
-Terminates the python script, the optional end callback is invoked if specified. A kill signal may be provided by `signal`, if `signal` is not specified SIGTERM is sent.
+Terminates the python script. A kill signal may be provided by `signal`, if `signal` is not specified SIGTERM is sent.
 
 #### event: `message`
 

index.ts

@@ -37,6 +37,9 @@ function getRandomInt(){
 }
 
 export interface Options extends SpawnOptions{
+    /**
+     * if binary is enabled message and stderr events will not be emitted
+     */
     mode?: 'text'|'json'|'binary'
     formatter?: (param:string)=>any
     parser?: (param:string)=>any
@@ -147,12 +150,14 @@ export class PythonShell extends EventEmitter{
         }
 
         // listen to stderr and emit errors for incoming data
+        if (this.stderrParser && this.stderr) {
+            this.stderr.on('data', this.receiveStderr.bind(this));
+        }
+
         if (this.stderr) {
             this.stderr.on('data', function (data) {
-                errorData += ''+data;
-                self.receiveStderr(data);
+                errorData += '' + data;
             });
-
             this.stderr.on('end', function(){
                 self.stderrHasEnded = true;
                 terminateIfNeeded();
@@ -229,7 +234,6 @@ export class PythonShell extends EventEmitter{
 
     /**
 	 * checks syntax without executing code
-	 * @param {string} code
 	 * @returns {Promise} rejects w/ stderr if syntax failure
 	 */
 	static async checkSyntax(code:string){
@@ -251,7 +255,6 @@ export class PythonShell extends EventEmitter{
 
 	/**
 	 * checks syntax without executing code
-	 * @param {string} filePath
 	 * @returns {Promise} rejects w/ stderr if syntax failure
 	 */
 	static async checkSyntaxFile(filePath:string){
@@ -343,7 +346,6 @@ export class PythonShell extends EventEmitter{
     /**
      * Sends a message to the Python shell through stdin
      * Override this method to format data to be sent to the Python process
-     * @param {string|Object} data The message to send
      * @returns {PythonShell} The same instance for chaining calls
      */
     send(message:string|Object) {
@@ -399,7 +401,8 @@ export class PythonShell extends EventEmitter{
     }
 
     /**
-     * Closes the stdin stream, which should cause the process to finish its work and close
+     * Closes the stdin stream. Unless python is listening for stdin in a loop 
+     * this should cause the process to finish its work and close.
      * @returns {PythonShell} The same instance for chaining calls
      */
     end(callback:(err:PythonShellError, exitCode:number,exitSignal:string)=>any) {
@@ -411,12 +414,21 @@ export class PythonShell extends EventEmitter{
     };
 
     /**
-     * Closes the stdin stream, which should cause the process to finish its work and close
+     * Sends a kill signal to the process
      * @returns {PythonShell} The same instance for chaining calls
      */
-    terminate(signal?:string) {
+    kill(signal?: NodeJS.Signals) {
         this.childProcess.kill(signal);
         this.terminated = true;
         return this;
     };
+
+    /**
+     * Alias for kill.
+     * @deprecated
+     */
+    terminate(signal?: NodeJS.Signals) {
+        // todo: remove this next breaking release
+        return this.kill(signal)
+    }
 };

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "python-shell",
-  "version": "1.0.8",
+  "version": "2.0.0",
   "description": "Run Python scripts from Node.js with simple (but efficient) inter-process communication through stdio",
   "keywords": [
     "python"
@@ -13,7 +13,7 @@
   "dependencies": {},
   "devDependencies": {
     "@types/mocha": "^5.2.5",
-    "@types/node": "^9.3.0",
+    "@types/node": "^10.5.2",
     "mocha": "^5.2.0",
     "mocha-appveyor-reporter": "^0.4.0",
     "should": "^13.2.1",

package.json

@@ -150,6 +150,14 @@ describe('PythonShell', function () {
                 done();
             });
         });
+        it('should run the script and fail with an extended stack trace even when mode is binary', function (done) {
+            PythonShell.run('error.py', {mode: "binary"}, function (err, results) {
+                err.should.be.an.Error;
+                err.exitCode.should.be.exactly(1);
+                err.stack.should.containEql('----- Python Traceback -----');
+                done();
+            });
+        });
         it('should run multiple scripts and fail with an extended stack trace for each of them', function (done) {
             let numberOfTimesToRun = 5;
             for (let i = 0; i < numberOfTimesToRun; i++) {
@@ -348,7 +356,8 @@ describe('PythonShell', function () {
                 mode: 'binary'
             });
             pyshell.receive = function () {
-                throw new Error('should not emit messages in binary mode');
+                done('should not emit messages in binary mode');
+                return undefined
             };
             pyshell.end(done);
         });
@@ -385,12 +394,12 @@ describe('PythonShell', function () {
             }).send('hello').send('world').end(done);
         });
         it('should not be invoked when mode is "binary"', function (done) {
-            let pyshell = new PythonShell('echo_args.py', {
-                args: ['hello', 'world'],
+            let pyshell = new PythonShell('stderrLogging.py', {
                 mode: 'binary'
             });
             pyshell.receiveStderr = function () {
-                throw new Error('should not emit stderr in binary mode');
+                done('should not emit stderr in binary mode');
+                return undefined
             };
             pyshell.end(done);
         });