feat: Retry initial connect call

docs/supportability-metrics.md

@@ -439,3 +439,11 @@ EventBuffer/soft_navigations/Dropped/Bytes
 * rrweb/node/3/bytes
 <!-- node type 4 = Meta -->
 * rrweb/node/4/bytes
+
+### Harvester
+<!-- Harvester retried a harvest -->
+* 'Harvester/Retry/Attempted/<feature_name>'
+<!-- Retry failed codes (dynamic) -->
+* 'Harvester/Retry/Failed/<code>'
+<!-- Retry succeeded codes (dynamic) -->
+* 'Harvester/Retry/Succeeded/<code>'

src/common/harvest/harvester.js

@@ -18,8 +18,10 @@ import { getSubmitMethod, xhr as xhrMethod, xhrFetch as fetchMethod } from '../u
 import { activatedFeatures } from '../util/feature-flags'
 import { dispatchGlobalEvent } from '../dispatch/global-event'
 
-const RETRY_FAILED = 'Harvester/Retry/Failed/'
-const RETRY_SUCCEEDED = 'Harvester/Retry/Succeeded/'
+const RETRY = 'Harvester/Retry/'
+const RETRY_ATTEMPTED = RETRY + 'Attempted/'
+const RETRY_FAILED = RETRY + 'Failed/'
+const RETRY_SUCCEEDED = RETRY + 'Succeeded/'
 
 export class Harvester {
   #started = false
@@ -86,7 +88,9 @@ export class Harvester {
      */
     function cbFinished (result) {
       if (aggregateInst.harvestOpts.prevAttemptCode) { // this means we just retried a harvest that last failed
-        handle(SUPPORTABILITY_METRIC_CHANNEL, [(result.retry ? RETRY_FAILED : RETRY_SUCCEEDED) + aggregateInst.harvestOpts.prevAttemptCode], undefined, FEATURE_NAMES.metrics, aggregateInst.ee)
+        const reportSM = (message) => handle(SUPPORTABILITY_METRIC_CHANNEL, [message], undefined, FEATURE_NAMES.metrics, aggregateInst.ee)
+        reportSM(RETRY_ATTEMPTED + aggregateInst.featureName)
+        reportSM((result.retry ? RETRY_FAILED : RETRY_SUCCEEDED) + aggregateInst.harvestOpts.prevAttemptCode)
         delete aggregateInst.harvestOpts.prevAttemptCode // always reset last observation so we don't falsely report again next harvest
         // In case this re-attempt failed again, that'll be handled (re-marked again) next.
       }
@@ -271,7 +275,7 @@ function baseQueryString (agentRef, qs, endpoint) {
     param('v', VERSION),
     transactionNameParam(),
     param('ct', agentRef.runtime.customTransaction),
-    '&rst=' + now(),
+    param('rst', now(), qs),
     '&ck=0', // ck param DEPRECATED - still expected by backend
     '&s=' + (session?.state.value || '0'), // the 0 id encaps all untrackable and default traffic
     param('ref', ref),

src/features/page_view_event/aggregate/index.js

@@ -27,6 +27,9 @@ export class Aggregate extends AggregateBase {
     this.firstByteToWindowLoad = 0 // our "frontend" duration
     this.firstByteToDomContent = 0 // our "dom processing" duration
 
+    this.payload = undefined // buffer this at harvest time to allow for an easy retry if the request fails
+    this.retries = 0
+
     if (!isValid(agentRef.info)) {
       this.ee.abort()
       return warn(43)
@@ -52,9 +55,14 @@ export class Aggregate extends AggregateBase {
    *
    * @param {Function} cb A function to run once the RUM call has finished - Defaults to activateFeatures
    * @param {*} customAttributes custom attributes to attach to the RUM call - Defaults to info.js
-   * @param {*} target The target to harvest to
    */
-  sendRum (customAttributes = this.agentRef.info.jsAttributes, target = { licenseKey: this.agentRef.info.licenseKey, applicationID: this.agentRef.info.applicationID }) {
+  sendRum (customAttributes = this.agentRef.info.jsAttributes) {
+    if (this.blocked) return
+    /** if we are "retrying", this is the state the feature will be in */
+    if (this.payload) {
+      this.triggerHarvestFor(this.payload)
+      return
+    }
     const info = this.agentRef.info
     const measures = {}
 
@@ -107,24 +115,30 @@ export class Aggregate extends AggregateBase {
     queryParameters.fp = firstPaint.current.value
     queryParameters.fcp = firstContentfulPaint.current.value
 
+    this.rumStartTime = now()
     const timeKeeper = this.agentRef.runtime.timeKeeper
     if (timeKeeper?.ready) {
-      queryParameters.timestamp = Math.floor(timeKeeper.correctRelativeTimestamp(now()))
+      queryParameters.timestamp = Math.floor(timeKeeper.correctRelativeTimestamp(this.rumStartTime))
+    } else {
+      queryParameters.rst = this.rumStartTime // we set this here in the feature instead of in the harvester to allow for a future retry to use the original RST
     }
 
-    this.rumStartTime = now()
+    this.payload ??= { qs: queryParameters, body }
+
+    this.triggerHarvestFor(this.payload)
+  }
 
+  triggerHarvestFor (payload) {
     this.agentRef.runtime.harvester.triggerHarvestFor(this, {
       directSend: {
-        target,
-        payload: { qs: queryParameters, body }
+        payload
       },
       needResponse: true,
       sendEmptyBody: true
     })
   }
 
-  postHarvestCleanup ({ status, responseText, xhr }) {
+  postHarvestCleanup ({ status, responseText, xhr, retry }) {
     const rumEndTime = now()
     let app, flags
     try {
@@ -135,9 +149,13 @@ export class Aggregate extends AggregateBase {
     }
 
     if (status >= 400 || status === 0) {
-      warn(18, status)
-      // Adding retry logic for the rum call will be a separate change; this.blocked will need to be changed since that prevents another triggerHarvestFor()
-      this.ee.abort()
+      if (retry && this.retries++ < 1) { // Only retry once
+        setTimeout(() => this.sendRum(), 5000) // Retry sending the RUM event after 5 seconds
+      } else {
+        warn(18, status)
+        this.blocked = true
+        this.ee.abort()
+      }
       return
     }
 
@@ -155,6 +173,7 @@ export class Aggregate extends AggregateBase {
       }
     } catch (error) {
       this.ee.abort()
+      this.blocked = true
       warn(17, error)
       return
     }

tests/specs/rum/retry-harvesting.e2e.js

@@ -26,10 +26,11 @@ describe('rum retry harvesting', () => {
   })
 
   afterEach(async () => {
+    await browser.testHandle.clearScheduledReplies('bamServer')
     await browser.destroyAgentSession(browser.testHandle)
   })
 
-  ;[400, 404, 408, 429, 500, 502, 503, 504, 512].forEach(statusCode => {
+  ;[400, 404].forEach(statusCode => {
     it(`should not retry rum and should not continue harvesting when request statusCode is ${statusCode}`, async () => {
       await browser.testHandle.scheduleReply('bamServer', {
         test: testRumRequest,
@@ -82,45 +83,27 @@ describe('rum retry harvesting', () => {
       expect(traceHarvests.length).toEqual(0)
       expect(errorMetricsHarvests.length).toEqual(0)
       expect(replaysHarvests.length).toEqual(0)
+    })
+  })
 
-      ;[
-        rumHarvests,
-        timingEventsHarvests,
-        ajaxEventsHarvests,
-        ajaxMetricsHarvests,
-        insightsHarvests,
-        interactionEventsHarvests,
-        traceHarvests,
-        errorMetricsHarvests,
-        replaysHarvests
-      ] = await Promise.all([
-        rumCapture.waitForResult({ timeout: 10000 }),
-        timingEventsCapture.waitForResult({ timeout: 10000 }),
-        ajaxEventsCapture.waitForResult({ timeout: 10000 }),
-        ajaxMetricsCapture.waitForResult({ timeout: 10000 }),
-        insightsCapture.waitForResult({ timeout: 10000 }),
-        interactionEventsCapture.waitForResult({ timeout: 10000 }),
-        traceCapture.waitForResult({ timeout: 10000 }),
-        errorMetricsCapture.waitForResult({ timeout: 10000 }),
-        replaysCapture.waitForResult({ timeout: 10000 }),
-        browser.url(await browser.testHandle.assetURL('/'))
-      ])
-
-      if (statusCode === 408) {
-        // Browsers automatically retry requests with status code 408
-        expect(rumHarvests.length).toBeLessThan(5)
-      } else {
-        expect(rumHarvests.length).toEqual(1)
-      }
+  ;[408, 429, 500, 502, 503, 504, 512].forEach(statusCode => {
+    it(`should retry rum and subsequent features should harvest when request statusCode is ${statusCode}`, async () => {
+      await browser.testHandle.scheduleReply('bamServer', {
+        test: testRumRequest,
+        permanent: false, // should only fail the first time
+        statusCode,
+        body: ''
+      })
 
-      expect(timingEventsHarvests.length).toEqual(0)
-      expect(ajaxEventsHarvests.length).toEqual(0)
-      expect(ajaxMetricsHarvests.length).toEqual(0)
-      expect(insightsHarvests.length).toEqual(0)
-      expect(interactionEventsHarvests.length).toEqual(0)
-      expect(traceHarvests.length).toEqual(0)
-      expect(errorMetricsHarvests.length).toEqual(0)
-      expect(replaysHarvests.length).toEqual(0)
+      const [[rumHarvest1, rumHarvest2]] = await Promise.all([
+        rumCapture.waitForResult({ totalCount: 2 }), // retry happens in 5 seconds after failure
+        timingEventsCapture.waitForResult({ totalCount: 1 }), // a subsequent feature should then still harvest here after first retry
+        browser.url(await browser.testHandle.assetURL('obfuscate-pii.html'))
+      ])
+      expect(rumHarvest1.request.query).toEqual(rumHarvest2.request.query)
+      expect(rumHarvest1.request.body).toEqual(rumHarvest2.request.body)
+      expect(rumHarvest1.reply.statusCode).toEqual(statusCode)
+      expect(rumHarvest2.reply.statusCode).toEqual(200)
     })
   })
 })