metrics: merge on failed transmission.

There's been a regression lurking for a while now that only got
exercised when the agent switched to Object.keys from an old-style
for...in enumeration of the properties on the object. This exposed
that the object being passed to Agent.prototype.mergeMetrics was not, in
fact, a metrics object, but the pre-serialization JSON array expected by
the collector.

Also added nock because Sinon doesn't have a way to easily mock HTTP
requests from Node. Nock's nice! Thanks, Pedro!

lib/agent.js

@@ -340,7 +340,8 @@ Agent.prototype.submitTransactionSampleData = function (encoded) {
  * @param {Transaction} transaction A finished transaction.
  */
 Agent.prototype.mergeTransaction = function (transaction) {
-  this.mergeMetrics(transaction.metrics);
+  this.metrics.merge(transaction.metrics);
+  this.emit('metricsMerged');
 };
 
 /**
@@ -349,13 +350,15 @@ Agent.prototype.mergeTransaction = function (transaction) {
  * and am not holding a reference to the very first metrics object created
  * upon instantiation.
  *
- * @param {Metrics} metrics The failed metrics submission to be aggregated into
- *                          the current Metrics instance.
+ * @param {Metrics} data The data object to be serialized and
+ *                       sent to the collector. The metrics will always be
+ *                       the fourth element.
+ * @param {Error} error The error returned from the collector.
  */
-Agent.prototype.mergeMetrics = function (metrics, error) {
-  if (error) logger.warn(error, "Merging metrics from last harvest cycle because:");
+Agent.prototype.mergeMetrics = function (data, error) {
+  logger.warn(error, "Merging metrics from last harvest cycle because:");
 
-  this.metrics.merge(metrics);
+  this.metrics.merge(data[3]);
   this.emit('metricsMerged');
 };
 

package.json

@@ -58,7 +58,8 @@
     "q": "*",
     "qx": "*",
     "nave": "*",
-    "should": "*"
+    "should": "*",
+    "nock": "*"
   },
   "repository": {
     "type": "git",

test/integration/connection-failure-503.tap.js

@@ -0,0 +1,78 @@
+'use strict';
+
+var path  = require('path')
+  , test  = require('tap').test
+  , nock  = require('nock')
+  , dns   = require ('dns')
+  , Agent = require(path.join(__dirname, '..', '..', 'lib', 'agent.js'))
+  , Transaction = require(path.join(__dirname, '..', '..', 'lib', 'transaction.js'))
+  ;
+
+test("harvesting with a mocked collector that returns 503", function (t) {
+  nock.disableNetConnect();
+
+  dns.lookup('collector.newrelic.com', function (error, collector) {
+    if (error) {
+      t.fail(error);
+      t.end();
+    }
+
+    var RUN_ID      = 1337
+      , url         = 'http://' + collector
+      , agent       = new Agent()
+      , transaction = new Transaction(agent)
+      ;
+
+    function path(method, runID) {
+      var fragment = '/agent_listener/invoke_raw_method?' +
+        'marshal_format=json&protocol_version=9&' +
+        'license_key=license%20key%20here&method=' + method;
+
+      if (runID) fragment += '&run_id=' + runID;
+
+      return fragment;
+    }
+
+    var redirect = nock(url).post(path('get_redirect_host'))
+                     .reply(200, {return_value : "collector.newrelic.com"});
+
+    var handshake = nock(url).post(path('connect'))
+                      .reply(200, {return_value : {agent_run_id : 1337}});
+
+    var sendMetrics = nock(url).post(path('metric_data', RUN_ID)).reply(503)
+      , sendErrors  = nock(url).post(path('error_data', RUN_ID)).reply(503)
+      , sendTrace   = nock(url).post(path('transaction_sample_data', RUN_ID)).reply(503)
+      , shutdown    = nock(url).post(path('shutdown', RUN_ID)).reply(503)
+      ;
+
+    // needs to be created to create the connection attribute
+    agent.start();
+
+    agent.connection.on('connect', function () {
+      // disable the default harvester
+      clearInterval(agent.harvesterHandle);
+
+      agent.connection.on('transactionSampleDataError', function () {
+        t.ok(redirect.isDone(),    "requested redirect");
+        t.ok(handshake.isDone(),   "got handshake");
+        t.ok(sendMetrics.isDone(), "tried to send metrics");
+        t.ok(sendErrors.isDone(),  "tried to send errors");
+        t.ok(sendTrace.isDone(),   "tried to send transaction trace");
+
+        agent.connection.on('shutdownError', function () {
+          t.ok(shutdown.isDone(), "tried to send shutdown");
+
+          t.end();
+        });
+
+        agent.stop();
+      });
+
+      // need sample data to give the harvest cycle something to send
+      agent.errors.add(transaction, new Error('test error'));
+      agent.traces.trace = transaction.getTrace();
+
+      agent.harvest();
+    });
+  });
+});