From 485d87ab0a377cf6312fe718be6874aa0785e390 Mon Sep 17 00:00:00 2001
From: Larry Myers <larry@larrymyers.com>
Date: Tue, 7 Jun 2016 15:23:32 -0500
Subject: [PATCH] fix: add defensive checks to safely handle browser
 disconnects.

This is a common occurrence when integrating with SauceLabs or BrowserStack.

* Add safety check to handle the case where a browser disconnects and
specSuccess/specSkipped/specFailure will throw an Error due to
suites[browser.id] returning undefined.
* Add safety check to handle the case where a browser disconnects and
onBrowserComplete receives a result object without total or failed values.
Causes XMLAttribute in xmlbuilder to throw an error.
* Add tests to validate new behavior works correctly and that no errors are thrown.
---
 index.js              | 12 +++++++++---
 test/reporter.spec.js | 24 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/index.js b/index.js
index d49edea..8363fbd 100644
--- a/index.js
+++ b/index.js
@@ -123,9 +123,9 @@ var JUnitReporter = function (baseReporterDecorator, config, logger, helper, for
       return // don't die if browser didn't start
     }
 
-    suite.att('tests', result.total)
+    suite.att('tests', result.total ? result.total : 0)
     suite.att('errors', result.disconnected || result.error ? 1 : 0)
-    suite.att('failures', result.failed)
+    suite.att('failures', result.failed ? result.failed : 0)
     suite.att('time', (result.netTime || 0) / 1000)
 
     suite.ele('system-out').dat(allMessages.join() + '\n')
@@ -140,7 +140,13 @@ var JUnitReporter = function (baseReporterDecorator, config, logger, helper, for
   }
 
   this.specSuccess = this.specSkipped = this.specFailure = function (browser, result) {
-    var spec = suites[browser.id].ele('testcase', {
+    var testsuite = suites[browser.id]
+
+    if (!testsuite) {
+      return
+    }
+
+    var spec = testsuite.ele('testcase', {
       name: nameFormatter(browser, result),
       time: ((result.time || 0) / 1000),
       classname: (typeof classNameFormatter === 'function' ? classNameFormatter : getClassName)(browser, result)
diff --git a/test/reporter.spec.js b/test/reporter.spec.js
index be8c26a..0ddcfc3 100644
--- a/test/reporter.spec.js
+++ b/test/reporter.spec.js
@@ -80,4 +80,28 @@ describe('JUnit reporter', function () {
     var writtenXml = fakeFs.writeFile.firstCall.args[1]
     expect(writtenXml).to.have.string('testcase name="Sender using it get request should not fail"')
   })
+
+  it('should safely handle missing suite browser entries when specSuccess fires', function () {
+    reporter.onRunStart([])
+
+    // don't try to call null.ele()
+    expect(reporter.specSuccess.bind(reporter, {id: 1}, {})).to.not.throw(TypeError)
+  })
+
+  it('should safely handle invalid test result objects when onBrowserComplete fires', function () {
+    var badBrowserResult = {
+      id: 'Android_4_1_2',
+      name: 'Android',
+      fullName: 'Android 4.1.2',
+      lastResult: {
+        error: true,
+        netTime: 0
+      }
+    }
+
+    reporter.onRunStart([ badBrowserResult ])
+
+    // never pass a null value to XMLAttribute via xmlbuilder attr()
+    expect(reporter.onBrowserComplete.bind(reporter, badBrowserResult)).not.to.throw(Error)
+  })
 })