fix: Undefined stns in blob Trace (#1039)

Author: cwli24

src/features/session_trace/aggregate/index.js

@@ -116,6 +116,7 @@ export class Aggregate extends AggregateBase {
 
     /** Get the ST nodes from the traceStorage buffer.  This also returns helpful metadata about the payload. */
     const { stns, earliestTimeStamp, latestTimeStamp } = this.traceStorage.takeSTNs()
+    if (!stns) return // there are no trace nodes
     if (options.retry) {
       this.sentTrace = stns
     }

src/features/session_trace/aggregate/trace/storage.js

@@ -274,8 +274,7 @@ export class TraceStorage {
   // Ajax (FEATURE) events--XML & fetches--pipes into ST here.
   storeXhrAgg (type, name, params, metrics) {
     if (type !== 'xhr') return
-    this.storeSTN(new TraceNode('Ajax', metrics.time, metrics.time + metrics.duration, `${params.status} ${params.method}: ${params.host}${params.pathname
-    }`))
+    this.storeSTN(new TraceNode('Ajax', metrics.time, metrics.time + metrics.duration, `${params.status} ${params.method}: ${params.host}${params.pathname}`, 'ajax'))
   }
 
   restoreNode (name, listOfSTNodes) {

tests/components/session_trace/index.test.js

@@ -17,7 +17,8 @@ jest.mock('../../../src/common/config/config', () => ({
   }),
   isConfigured: jest.fn().mockReturnValue(true),
   getRuntime: jest.fn().mockReturnValue({
-    session: { state: { value: 'sessionID' }, write: jest.fn() }
+    session: { state: { value: 'sessionID' }, write: jest.fn() },
+    timeKeeper: { ready: true, correctedOriginTime: 0, convertRelativeTimestamp: jest.fn() }
   })
 }))
 jest.mock('../../../src/common/constants/runtime', () => ({
@@ -56,51 +57,57 @@ describe('session trace', () => {
 
     document.dispatchEvent(new CustomEvent('DOMContentLoaded')) // simulate natural browser event
     window.dispatchEvent(new CustomEvent('load')) // load is actually ignored by Trace as it should be passed by the PVT feature, so it should not be in payload
-    await traceInstrument.onAggregateImported
     ee.get('abcd').emit('rumresp-st', [1])
     ee.get('abcd').emit('rumresp-sts', [1])
-    const traceAggregate = traceInstrument.featAggregate
     traceAggregate.traceStorage.storeXhrAgg('xhr', '[200,null,null]', { method: 'GET', status: 200 }, { rxSize: 770, duration: 99, cbTime: 0, time: 217 }) // fake ajax data
     traceAggregate.traceStorage.processPVT('fi', 30, { fid: 8 }) // fake pvt data
-    setTimeout(() => {
-      const payload = traceAggregate.prepareHarvest()
-      let res = payload.body
 
-      let node = res.filter(node => node.n === 'DOMContentLoaded')[0]
-      expect(node).toBeTruthy()
-      expect(node.s).toBeGreaterThan(10) // that DOMContentLoaded node has start time
-      expect(node.o).toEqual('document') // that DOMContentLoaded origin is the document
-      node = res.filter(node => node.n === 'load' && (node.o === 'document' || node.o === 'window'))[0]
-      expect(node).toBeUndefined()
+    const payload = traceAggregate.prepareHarvest()
+    let res = payload.body
+
+    let node = res.filter(node => node.n === 'DOMContentLoaded')[0]
+    expect(node).toBeTruthy()
+    expect(node.s).toBeGreaterThan(10) // that DOMContentLoaded node has start time
+    expect(node.o).toEqual('document') // that DOMContentLoaded origin is the document
+    node = res.filter(node => node.n === 'load' && (node.o === 'document' || node.o === 'window'))[0]
+    expect(node).toBeUndefined()
+
+    let hist = res.filter(node => node.n === 'history.pushState')[1]
+    const originalPath = window.location.pathname
+    expect(hist.s).toEqual(hist.e) // that hist node has no duration
+    expect(hist.n).toEqual('history.pushState')
+    expect(hist.o).toEqual(`${originalPath}#bar`)
+    expect(hist.t).toEqual(`${originalPath}#foo`)
 
-      let hist = res.filter(node => node.n === 'history.pushState')[1]
-      const originalPath = window.location.pathname
-      expect(hist.s).toEqual(hist.e) // that hist node has no duration
-      expect(hist.n).toEqual('history.pushState')
-      expect(hist.o).toEqual(`${originalPath}#bar`)
-      expect(hist.t).toEqual(`${originalPath}#foo`)
+    let ajax = res.filter(node => node.t === 'ajax')[0]
+    expect(ajax.s).toBeLessThan(ajax.e) // that it has some duration
+    expect(ajax.n).toEqual('Ajax')
+    expect(ajax.t).toEqual('ajax')
 
-      let ajax = res.filter(node => node.t === 'ajax')[0]
-      expect(ajax.s).toBeLessThan(ajax.e) // that it has some duration
-      expect(ajax.n).toEqual('Ajax')
-      expect(ajax.t).toEqual('ajax')
+    let pvt = res.filter(node => node.n === 'fi')[0]
+    expect(pvt.o).toEqual('document')
+    expect(pvt.s).toEqual(pvt.e) // that FI has no duration
+    expect(pvt.t).toEqual('timing')
+    pvt = res.filter(node => node.n === 'fid')[0]
+    expect(pvt.o).toEqual('document')
+    expect(pvt.s).toEqual(30) // that FID has a duration relative to FI'
+    expect(pvt.e).toEqual(30 + 8)
+    expect(pvt.t).toEqual('event')
 
-      let pvt = res.filter(node => node.n === 'fi')[0]
-      expect(pvt.o).toEqual('document')
-      expect(pvt.s).toEqual(pvt.e) // that FI has no duration
-      expect(pvt.t).toEqual('timing')
-      pvt = res.filter(node => node.n === 'fid')[0]
-      expect(pvt.o).toEqual('document')
-      expect(pvt.s).toEqual(30) // that FID has a duration relative to FI'
-      expect(pvt.e).toEqual(30 + 8)
-      expect(pvt.t).toEqual('event')
+    let unknown = res.filter(n => n.o === 'unknown')
+    expect(unknown.length).toEqual(0) // no events with unknown origin
+  })
 
-      let unknown = res.filter(n => n.o === 'unknown')
-      expect(unknown.length).toEqual(0) // no events with unknown origin
-    }, 1000)
+  test('prepareHarvest returns undefined if there are no trace nodes', () => {
+    const spy = jest.spyOn(traceAggregate.traceStorage, 'takeSTNs')
+    let payload
+    expect(() => (payload = traceAggregate.prepareHarvest())).not.toThrow()
+    expect(spy).toHaveBeenCalled()
+    expect(payload).toBeUndefined()
+    spy.mockRestore()
   })
 
-  test('tracks previously stored events and processes them once per occurrence', () => {
+  test('tracks previously stored events and processes them once per occurrence', done => {
     document.addEventListener('visibilitychange', () => 1)
     document.addEventListener('visibilitychange', () => 2)
     document.addEventListener('visibilitychange', () => 3) // additional listeners should not generate additional nodes
@@ -113,12 +120,12 @@ describe('session trace', () => {
     }))
     expect(traceAggregate.traceStorage.prevStoredEvents.size).toEqual(1)
 
-    jest.advanceTimersToNextTimer(1) // this will increase perf.now by the default ~20ms to imitate some time gap
-    document.dispatchEvent(new Event('visibilitychange'))
-    expect(traceAggregate.traceStorage.trace.visibilitychange.length).toEqual(2)
-    expect(traceAggregate.traceStorage.trace.visibilitychange[0].s).not.toEqual(traceAggregate.traceStorage.trace.visibilitychange[1].s) // should not have same start times
-    expect(traceAggregate.traceStorage.prevStoredEvents.size).toEqual(2)
-
-    jest.useRealTimers()
+    setTimeout(() => { // some time gap
+      document.dispatchEvent(new Event('visibilitychange'))
+      expect(traceAggregate.traceStorage.trace.visibilitychange.length).toEqual(2)
+      expect(traceAggregate.traceStorage.trace.visibilitychange[0].s).not.toEqual(traceAggregate.traceStorage.trace.visibilitychange[1].s) // should not have same start times
+      expect(traceAggregate.traceStorage.prevStoredEvents.size).toEqual(2)
+      done()
+    }, 1)
   })
 })