fix: SPA feature respects ajax deny_list for fetch (#633)

newrelic · Aug 2, 2023 · ccfe510 · ccfe510
1 parent cff7bc4
commit ccfe510
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 29 deletions.
diff --git a/src/features/spa/aggregate/index.js b/src/features/spa/aggregate/index.js
@@ -415,7 +415,7 @@ export class Aggregate extends AggregateBase {
     register(FETCH_DONE, function (err, res) {
       var node = this[SPA_NODE]
       if (node) {
-        if (err) {
+        if (err || !shouldCollectEvent(this.params)) {
           node.cancel()
           return
         }

diff --git a/tests/assets/spa/ajax-deny-list.html b/tests/assets/spa/ajax-deny-list.html
@@ -19,6 +19,10 @@
         xhr.addEventListener('load', function () {}, true)
         xhr.send()
       }, true)
+
+      if (typeof fetch === 'function') {
+        fetch('/json').then(function () { fetch('/text') })
+      }
     </script>
   </head>
   <body>

diff --git a/tests/assets/spa/fetch-chunked.html b/tests/assets/spa/fetch-chunked.html
@@ -6,6 +6,7 @@
 <html>
   <head>
     <title>RUM Unit Test</title>
+    {init}
     {config}
     {loader}
   </head>

diff --git a/tests/assets/spa/fetch-simple.html b/tests/assets/spa/fetch-simple.html
@@ -6,6 +6,7 @@
 <html>
   <head>
     <title>RUM Unit Test</title>
+    {init}
     {config}
     {loader}
   </head>

diff --git a/tests/specs/xhr/deny-list.e2e.js b/tests/specs/xhr/deny-list.e2e.js
@@ -1,18 +1,79 @@
+import { supportsFetch } from '../../../tools/browser-matcher/common-matchers.mjs'
+
 describe('Ajax events to beacon endpoint', () => {
   it('not captured when blocked', async () => {
-    let url = await browser.testHandle.assetURL('spa/ajax-deny-list.html', { init: { ajax: { block_internal: true } } })
-    let nextAjaxReq = browser.testHandle.expectAjaxEvents(10000, true)
-    await browser.url(url)
+    const [ajaxEvents, interactionEvents] = await Promise.all([
+      browser.testHandle.expectAjaxEvents(10000, true),
+      browser.testHandle.expectInteractionEvents(),
+      browser.url(await browser.testHandle.assetURL('spa/ajax-deny-list.html', { init: { ajax: { block_internal: true } } }))
+    ])
 
-    await expect(nextAjaxReq).resolves.toBeUndefined()
+    expect(ajaxEvents).toBeUndefined()
+    expect(interactionEvents.request.body).not.toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        domain: expect.stringContaining('bam-test-1.nr-local.net'),
+        path: '/json',
+        type: 'ajax',
+        requestedWith: 'XMLHttpRequest'
+      }),
+      expect.objectContaining({
+        domain: expect.stringContaining('bam-test-1.nr-local.net'),
+        path: '/text',
+        type: 'ajax',
+        requestedWith: 'XMLHttpRequest'
+      }),
+      expect.objectContaining({
+        domain: expect.stringContaining('bam-test-1.nr-local.net'),
+        path: '/json',
+        type: 'ajax',
+        requestedWith: 'fetch'
+      }),
+      expect.objectContaining({
+        domain: expect.stringContaining('bam-test-1.nr-local.net'),
+        path: '/text',
+        type: 'ajax',
+        requestedWith: 'fetch'
+      })
+    ]))
   })
 
-  it('captured when allowed', async () => {
-    let url = await browser.testHandle.assetURL('spa/ajax-deny-list.html', { init: { ajax: { block_internal: false } } })
-    let nextAjaxReq = browser.testHandle.expectAjaxEvents(10000)
-    await browser.url(url)
+  it('is captured when not blocked', async () => {
+    const [ajaxEvents, interactionEvents] = await Promise.all([
+      browser.testHandle.expectAjaxEvents(),
+      browser.testHandle.expectInteractionEvents(),
+      browser.url(await browser.testHandle.assetURL('spa/ajax-deny-list.html', { init: { ajax: { block_internal: false } } }))
+    ])
 
-    let correctXhrEvent = (await nextAjaxReq)?.request.body.some(ajaxNode => ajaxNode.domain.startsWith('bam-test-1.nr-local.net') && ajaxNode.path === '/json')
-    expect(correctXhrEvent).toEqual(true)
+    const events = [...ajaxEvents.request.body, ...interactionEvents.request.body]
+    expect(events).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        domain: expect.stringContaining('bam-test-1.nr-local.net'),
+        path: '/json',
+        type: 'ajax',
+        requestedWith: 'XMLHttpRequest'
+      }),
+      expect.objectContaining({
+        domain: expect.stringContaining('bam-test-1.nr-local.net'),
+        path: '/text',
+        type: 'ajax',
+        requestedWith: 'XMLHttpRequest'
+      }),
+      ...(browserMatch(supportsFetch)
+        ? [
+            expect.objectContaining({
+              domain: expect.stringContaining('bam-test-1.nr-local.net'),
+              path: '/json',
+              type: 'ajax',
+              requestedWith: 'fetch'
+            }),
+            expect.objectContaining({
+              domain: expect.stringContaining('bam-test-1.nr-local.net'),
+              path: '/text',
+              type: 'ajax',
+              requestedWith: 'fetch'
+            })
+          ]
+        : [])
+    ]))
   })
 })
diff --git a/tools/wdio/plugins/browser-matcher.mjs b/tools/wdio/plugins/browser-matcher.mjs
@@ -14,10 +14,6 @@ export default class BrowserMatcher {
     this.#browserName = getBrowserName(capabilities)
     this.#browserVersion = getBrowserVersion(capabilities)
     this.#setupMochaGlobals()
-    global.withBrowsersMatching = (matcher) => {
-      log.warn('withBrowsersMatching() global deprecated, use it.withBrowsersMatching() or describe.withBrowsersMatching()')
-      return this.#browserMatchTest(matcher, global.it)
-    }
   }
 
   #setupMochaGlobals () {
@@ -44,29 +40,22 @@ export default class BrowserMatcher {
         globalIt = value
       }
     })
+
+    global.browserMatch = (matcher) => {
+      return !this.#wrapFnWithBrowserMatcher(matcher)
+    }
   }
 
   #extendMochaGlobal (originalGlobal) {
     Object.defineProperty(originalGlobal, 'withBrowsersMatching', {
       value: (matcher) => {
-        return this.#browserMatchTest(matcher, originalGlobal)
+        return this.#wrapFnWithBrowserMatcher(matcher, originalGlobal)
       }
     })
   }
 
-  #browserMatchTest (matcher, originalGlobal) {
-    let skip = false
-
-    if (Array.isArray(matcher) && matcher.length > 0) {
-      for (const indexedMatcher of matcher) {
-        if (!indexedMatcher.test(this.#browserName, this.#browserVersion)) {
-          skip = true
-          break
-        }
-      }
-    } else if (matcher && typeof matcher.test === 'function') {
-      skip = !matcher.test(this.#browserName, this.#browserVersion)
-    }
+  #wrapFnWithBrowserMatcher (matcher, originalGlobal) {
+    const skip = this.#browserMatchTest(matcher)
 
     return function (...args) {
       /*
@@ -83,4 +72,21 @@ export default class BrowserMatcher {
       }
     }
   }
+
+  #browserMatchTest (matcher) {
+    let skip = false
+
+    if (Array.isArray(matcher) && matcher.length > 0) {
+      for (const indexedMatcher of matcher) {
+        if (!indexedMatcher.test(this.#browserName, this.#browserVersion)) {
+          skip = true
+          break
+        }
+      }
+    } else if (matcher && typeof matcher.test === 'function') {
+      skip = !matcher.test(this.#browserName, this.#browserVersion)
+    }
+
+    return skip
+  }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -6,6 +6,7 @@ @@
     <html>
       <head>
         <title>RUM Unit Test</title>
+        {init}
         {config}
         {loader}
       </head>
@@ Expand Down @@