add retry count to URL when sending

src/__tests__/request-utils.test.ts

@@ -0,0 +1,34 @@
+import { _HTTPBuildQuery, _isUrlMatchingRegex } from '../request-utils'
+
+describe('request utils', () => {
+    describe('_HTTPBuildQuery', () => {
+        test.each([
+            ['builds query string', { x: 'y', a: 'b' }, 'x=y&a=b'],
+            ['skips undefined values', { x: 'y', a: undefined }, 'x=y'],
+            ['skips undefined keys', { x: 'y', a: 'b', undefined: 'c' }, 'x=y&a=b'],
+        ])('%s', (_name, formData, expected) => {
+            expect(_HTTPBuildQuery(formData)).toEqual(expected)
+        })
+    })
+    describe('_isUrlMatchingRegex', () => {
+        test.each([
+            ['match query params', 'https://example.com?name=something', '(\\?|\\&)(name.*)\\=([^&]+)', true],
+            [
+                'match query params with trailing slash',
+                'https://example.com/?name=something',
+                '(\\?|\\&)(name.*)\\=([^&]+)',
+                true,
+            ],
+            ['match subdomain wildcard', 'https://app.example.com', '(.*.)?example.com', true],
+            ['match route wildcard', 'https://example.com/something/test', 'example.com/(.*.)/test', true],
+            ['match domain', 'https://example.com', 'example.com', true],
+            ['match domain with protocol', 'https://example.com', 'https://example.com', true],
+            ['match domain with protocol and wildcard', 'https://example.com', 'https://(.*.)?example.com', true],
+            ['does not match query params', 'https://example.com', '(\\?|\\&)(name.*)\\=([^&]+)', false],
+            ['does not match route', 'https://example.com', 'example.com/test', false],
+            ['does not match domain', 'https://example.com', 'anotherone.com', false],
+        ])('%s', (_name, url, regex, expected) => {
+            expect(_isUrlMatchingRegex(url, regex)).toEqual(expected)
+        })
+    })
+})

src/__tests__/send-request.ts

@@ -10,7 +10,7 @@ jest.mock('../config', () => ({ DEBUG: false, LIB_VERSION: '1.23.45' }))
 describe('send-request', () => {
     describe('xhr', () => {
         let mockXHR: XMLHttpRequest
-        let xhrParams: () => XHRParams
+        let xhrParams: (overrides?: Partial<XHRParams>) => XHRParams
         let onXHRError: XHRParams['onXHRError']
         let checkForLimiting: XHRParams['onResponse']
         let xhrOptions: XHRParams['options']
@@ -29,26 +29,44 @@ describe('send-request', () => {
             onXHRError = jest.fn()
             checkForLimiting = jest.fn()
             xhrOptions = {}
-            xhrParams = () => ({
-                url: 'https://any.posthog-instance.com',
-                data: {},
-                headers: {},
-                options: xhrOptions,
-                callback: () => {},
-                retriesPerformedSoFar: undefined,
-                retryQueue: {
-                    enqueue: () => {},
-                } as Partial<XHRParams['retryQueue']> as XHRParams['retryQueue'],
-                onXHRError,
-                onResponse: checkForLimiting,
-            })
+            xhrParams = (overrides?: Partial<XHRParams>) => {
+                return {
+                    url: 'https://any.posthog-instance.com?ver=1.23.45',
+                    data: {},
+                    headers: {},
+                    options: xhrOptions,
+                    callback: () => {},
+                    retriesPerformedSoFar: undefined,
+                    retryQueue: {
+                        enqueue: () => {},
+                    } as Partial<XHRParams['retryQueue']> as XHRParams['retryQueue'],
+                    onXHRError,
+                    onResponse: checkForLimiting,
+                    ...overrides,
+                }
+            }
 
             // ignore TS complaining about us cramming a fake in here
             // eslint-disable-next-line @typescript-eslint/ban-ts-comment
             // @ts-ignore
             window.XMLHttpRequest = jest.fn(() => mockXHR) as unknown as XMLHttpRequest
         })
 
+        test('it adds the retry count to the URL', () => {
+            const retryCount = Math.floor(Math.random() * 100)
+            xhr(
+                xhrParams({
+                    retriesPerformedSoFar: retryCount,
+                    url: 'https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278',
+                })
+            )
+            expect(mockXHR.open).toHaveBeenCalledWith(
+                'GET',
+                `https://any.posthog-instance.com/?ver=1.23.45&ip=7&_=1698404857278&retry_count=${retryCount}`,
+                true
+            )
+        })
+
         describe('when xhr requests fail', () => {
             it('does not error if the configured onXHRError is not a function', () => {
                 onXHRError = 'not a function' as unknown as XHRParams['onXHRError']
@@ -100,7 +118,6 @@ describe('send-request', () => {
             // eslint-disable-next-line compat/compat
             expect(new URL(alteredURL).search).toContain('&ver=1.23.45')
         })
-
         it('adds i as 1 when IP in config', () => {
             const alteredURL = addParamsToURL(
                 posthogURL,
@@ -129,7 +146,6 @@ describe('send-request', () => {
             // eslint-disable-next-line compat/compat
             expect(new URL(alteredURL).search).toMatch(/_=\d+/)
         })
-
         it('does not add a query parameter if it already exists in the URL', () => {
             posthogURL = 'https://test.com/'
             const whenItShouldAddParam = addParamsToURL(
@@ -152,6 +168,41 @@ describe('send-request', () => {
             expect(whenItShouldNotAddParam).not.toContain('ver=1.23.45')
             expect(whenItShouldNotAddParam).toContain('ver=2')
         })
+
+        it('does not add the i query parameter if it already exists in the URL', () => {
+            posthogURL = 'https://test.com/'
+            expect(
+                addParamsToURL(
+                    'https://test.com/',
+                    {},
+                    {
+                        ip: true,
+                    }
+                )
+            ).toContain('ip=1')
+
+            expect(
+                addParamsToURL(
+                    'https://test.com/',
+                    {},
+                    {
+                        ip: false,
+                    }
+                )
+            ).toContain('ip=0')
+
+            expect(addParamsToURL('https://test.com/', {}, {})).toContain('ip=0')
+
+            const whenItShouldNotAddParam = addParamsToURL(
+                'https://test.com/decide/?ip=7',
+                {},
+                {
+                    ip: true,
+                }
+            )
+            expect(whenItShouldNotAddParam).not.toContain('ip=1')
+            expect(whenItShouldNotAddParam).toContain('ip=7')
+        })
     })
 
     describe('using property based testing to identify edge cases in encodePostData', () => {

src/request-utils.ts

@@ -1,4 +1,4 @@
-import { _each, _isNull, _isString, _isValidRegex, logger } from './utils'
+import { _each, _isNull, _isString, _isUndefined, _isValidRegex, logger } from './utils'
 
 const localDomains = ['localhost', '127.0.0.1']
 
@@ -13,6 +13,11 @@ export const _HTTPBuildQuery = function (formdata: Record<string, any>, arg_sepa
     const tph_arr: string[] = []
 
     _each(formdata, function (val, key) {
+        // the key might be literally the string undefined for e.g. if {undefined: 'something'}
+        if (_isUndefined(val) || _isUndefined(key) || key === 'undefined') {
+            return
+        }
+
         use_val = encodeURIComponent(val.toString())
         use_key = encodeURIComponent(key)
         tph_arr[tph_arr.length] = use_key + '=' + use_val

src/send-request.ts

@@ -18,7 +18,7 @@ export const addParamsToURL = (
         const params = halves[1].split('&')
         for (const p of params) {
             const key = p.split('=')[0]
-            if (args[key]) {
+            if (key in args) {
                 delete args[key]
             }
         }
@@ -70,6 +70,8 @@ export const xhr = ({
     timeout = 60000,
     onResponse,
 }: XHRParams) => {
+    url = addParamsToURL(url, { retry_count: retriesPerformedSoFar }, {})
+
     const req = new XMLHttpRequest()
     req.open(options.method || 'GET', url, true)