Remove IPC listener on timeout Fixes #161 (#162)

* Remove IPC listener on timeout.  And goodbye jasmime, electron-ava to the rescue. Fixes #161

* better naming and documentation #161

Author: codecounselor

lib/exportJob.js

@@ -23,6 +23,7 @@ const HTML_DPI = 96
 const MICRONS_INCH_RATIO = 25400
 const MAX_EVENT_WAIT = 10000
 const IPC_MAIN_CHANNEL_RENDER = 'READY_TO_RENDER'
+const eventPrefix = 'job.render.'
 
 const DEFAULT_OPTIONS = {
   closeWindow: true,
@@ -57,7 +58,12 @@ class ExportJob extends EventEmitter {
    *   the filesystem
    */
   constructor (input, output, args, options) {
-    super({ wildcard: true })
+    super({
+      // Allow listeners to provide wildcards
+      wildcard: true,
+      // displays the event name if maxListeners is reached for an event
+      verboseMemoryLeak: true
+    })
     this.jobId = uuid()
     this.input = _.isArray(input) ? input : [input]
     this.output = output
@@ -79,7 +85,6 @@ class ExportJob extends EventEmitter {
    * Render markdown or html to pdf
    */
   render (window) {
-    const eventPrefix = 'job.render.'
     this.emit(`${eventPrefix}start`)
 
     const win = this._launchBrowserWindow()
@@ -396,6 +401,7 @@ class ExportJob extends EventEmitter {
     if (this.args.disableCache) {
       loadOpts.extraHeaders += 'pragma: no-cache\n'
     }
+    this.emit(`${eventPrefix}loadurl`, { url: url })
     window.loadURL(wargs.urlWithArgs(url, {}), loadOpts)
   }
 
@@ -422,37 +428,55 @@ class ExportJob extends EventEmitter {
 
   _waitForBrowserEvent (waitForJSEvent, window, generateFunction) {
     const eventName = _.size(waitForJSEvent) > 0 ? waitForJSEvent : 'view-ready'
-    this._attachIPCListener(eventName, generateFunction)
-    this._executeJSListener(eventName, generateFunction, window)
+    const ipcListener = this._attachIPCListener(eventName, generateFunction)
+    this._executeJSListener(eventName, ipcListener, generateFunction, window)
   }
 
   /**
    * responsible for executing JS in the browser that will wait for the page
    * to emit an event before capturing the page.
    *
    * @param eventName
+   * @param ipcListener The listener for the ready event.  This needs cancelled
+   *   if there is a timeout before it the event is received
    * @param generateFunction
    * @param window
    * @private
    */
-  _executeJSListener (eventName, generateFunction, window) {
+  _executeJSListener (eventName, ipcListener, generateFunction, window) {
     // event.detail will only exist if a CustomEvent was emitted
     const cmd = `var ipcRenderer = require('electron').ipcRenderer
                  document.body.addEventListener('${eventName}', 
                    function(event) {
                      ipcRenderer.send('${IPC_MAIN_CHANNEL_RENDER}', '${this.jobId}', event.detail)
                    }
                  )`
-    // Don't let things hang forever
-    const timeout = setTimeout(() => {
-      this.emit('window.event.wait.timeout', {eventName: eventName})
-      generateFunction()
-    }, this.args.outputWait > 0 ? this.args.outputWait : MAX_EVENT_WAIT)
 
+    // Don't let a ready event hang, set a max timeout interval
+    const f = this._cancelReadyEvent.bind(this, eventName, ipcListener, generateFunction)
+    const maxWait = this.args.outputWait > 0 ? this.args.outputWait : MAX_EVENT_WAIT
+    const timeout = setTimeout(f, maxWait)
+
+    // clear the timeout as soon as we get the ready event from the browser
     this.once('window.event.wait.end', () => clearTimeout(timeout))
+
     window.webContents.executeJavaScript(cmd)
   }
 
+  /**
+   * Invoked when a ready event has not been received before the max timeout is reached
+   * @param eventName The eventName provided by the client
+   * @param ipcListener The ipcMain listener waiting for the IPC_MAIN_CHANNEL_RENDER
+   * event from the renderer process
+   * @param generateFunction A callback function to invoke to capture the window
+   * @private
+   */
+  _cancelReadyEvent (eventName, ipcListener, generateFunction) {
+    this.emit('window.event.wait.timeout', {eventName: eventName})
+    electron.ipcMain.removeListener(IPC_MAIN_CHANNEL_RENDER, ipcListener)
+    generateFunction()
+  }
+
   /**
    * Listen for the browser to emit the READY_TO_RENDER event and when it does
    * emit our own event so the max load timer is removed.
@@ -478,6 +502,7 @@ class ExportJob extends EventEmitter {
       }
     }
     electron.ipcMain.on(IPC_MAIN_CHANNEL_RENDER, listener)
+    return listener
   }
 
   /**

package.json

@@ -7,9 +7,7 @@
     "fix": "standard --fix",
     "lint": "standard",
     "unit-test": "ava | tap-diff",
-    "test": "npm run lint && ava | tap-diff",
-    "\\": "This effectively runs jasmine tests as an electron application (but it doesn't work for Ava)",
-    "integration-test": "electron node_modules/.bin/jasmine"
+    "test": "npm run fix && ava **/*-test.js | tap-diff && electron-ava --tap **/*-test-it.js | tap-diff"
   },
   "bin": {
     "electron-pdf": "cli.js"
@@ -36,6 +34,7 @@
   "homepage": "https://github.com/fraserxu/electron-pdf",
   "devDependencies": {
     "ava": "^0.18.0",
+    "electron-ava": "^0.3.0",
     "jasmine": "^2.5.2",
     "standard": "^8.4.0",
     "tap-diff": "^0.1.1",

spec/index-test.js

@@ -0,0 +1,21 @@
+import { test } from 'ava'
+
+import electron from 'electron'
+
+import ExportJob from '../lib/exportJob'
+
+const args = {}
+const options = {
+  pageSize: 'Letter'
+}
+let job = new ExportJob(['input'], 'output.pdf', args, options)
+
+// Ready Event, wait times
+test('_readyEventTimeout removes ipc listener', t => {
+  const generateFunction = () => {}
+  const ipcListener = job._attachIPCListener('myEvent', generateFunction)
+  t.is(electron.ipcMain.listenerCount('READY_TO_RENDER'), 1)
+  job._cancelReadyEvent('myEvent', ipcListener, generateFunction)
+  t.is(electron.ipcMain.listenerCount('READY_TO_RENDER'), 0)
+})
+

spec/source-test.js

@@ -1,5 +1,7 @@
 import {test} from 'ava'
+
 import _ from 'lodash'
+
 import validator from 'validator'
 
 import ExportJob from '../lib/exportJob'