feat: Move all logs everything over to logger (#830)

PostHog · Oct 11, 2023 · 00a6a4a · 00a6a4a
1 parent ab2ac71
commit 00a6a4a
Show file tree

Hide file tree

Showing 24 changed files with 116 additions and 303 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -9,6 +9,7 @@ const rules = {
     '@typescript-eslint/no-unused-vars': ['error'],
     'no-prototype-builtins': 'off',
     'no-empty': 'off',
+    'no-console': 'error',
 }
 
 const extend = [
@@ -51,7 +52,7 @@ module.exports = {
             // but excluding the 'plugin:compat/recommended' rule
             // we don't mind using the latest features in our tests
             extends: extend.filter((s) => s !== 'plugin:compat/recommended'),
-            rules,
+            rules: rules.filter((s) => s !== 'no-console'),
         },
     ],
     root: true,

diff --git a/react/src/context/PostHogProvider.tsx b/react/src/context/PostHogProvider.tsx
@@ -1,3 +1,4 @@
+/* eslint-disable no-console */
 import posthogJs, { PostHogConfig } from 'posthog-js'
 
 import React, { useMemo } from 'react'
@@ -17,13 +18,13 @@ export function PostHogProvider({
     const posthog = useMemo(() => {
         if (client && apiKey) {
             console.warn(
-                'You have provided both a client and an apiKey to PostHogProvider. The apiKey will be ignored in favour of the client.'
+                '[PostHog.js] You have provided both a client and an apiKey to PostHogProvider. The apiKey will be ignored in favour of the client.'
             )
         }
 
         if (client && options) {
             console.warn(
-                'You have provided both a client and options to PostHogProvider. The options will be ignored in favour of the client.'
+                '[PostHog.js] You have provided both a client and options to PostHogProvider. The options will be ignored in favour of the client.'
             )
         }
 
@@ -33,7 +34,7 @@ export function PostHogProvider({
 
         if (apiKey) {
             if (posthogJs.__loaded) {
-                console.warn('posthog was already loaded elsewhere. This may cause issues.')
+                console.warn('[PostHog.js] was already loaded elsewhere. This may cause issues.')
             }
             posthogJs.init(apiKey, options)
         }

diff --git a/src/__tests__/decide.js b/src/__tests__/decide.js
@@ -183,12 +183,13 @@ describe('Decide', () => {
 
         it('Make sure receivedFeatureFlags is not called if the decide response fails', () => {
             given('decideResponse', () => ({ status: 0 }))
+            window.POSTHOG_DEBUG = true
             console.error = jest.fn()
 
             given.subject()
 
             expect(given.posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled()
-            expect(console.error).toHaveBeenCalledWith('Failed to fetch feature flags from PostHog.')
+            expect(console.error).toHaveBeenCalledWith('[PostHog.js]', 'Failed to fetch feature flags from PostHog.')
         })
 
         it('Make sure receivedFeatureFlags is called with empty if advanced_disable_feature_flags_on_first_load is set', () => {
@@ -221,13 +222,14 @@ describe('Decide', () => {
         })
 
         it('does not run site apps code if not opted in', () => {
+            window.POSTHOG_DEBUG = true
             given('config', () => ({ api_host: 'https://test.com', opt_in_site_apps: false, persistence: 'memory' }))
             given('decideResponse', () => ({ siteApps: [{ id: 1, url: '/site_app/1/tokentoken/hash/' }] }))
             expect(() => {
                 given.subject()
             }).toThrow(
                 // throwing only in tests, just an error in production
-                'Unexpected console.error: PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.'
+                'Unexpected console.error: [PostHog.js],PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.'
             )
         })
     })

diff --git a/src/__tests__/featureflags.js b/src/__tests__/featureflags.js
@@ -67,20 +67,23 @@ describe('featureflags', () => {
     })
 
     it('should warn if decide endpoint was not hit and no flags exist', () => {
+        window.POSTHOG_DEBUG = true
         given.featureFlags.instance.decideEndpointWasHit = false
         given.instance.persistence.unregister('$enabled_feature_flags')
         given.instance.persistence.unregister('$active_feature_flags')
 
         expect(given.featureFlags.getFlags()).toEqual([])
         expect(given.featureFlags.isFeatureEnabled('beta-feature')).toEqual(undefined)
         expect(window.console.warn).toHaveBeenCalledWith(
+            '[PostHog.js]',
             'isFeatureEnabled for key "beta-feature" failed. Feature flags didn\'t load in time.'
         )
 
         window.console.warn.mockClear()
 
         expect(given.featureFlags.getFeatureFlag('beta-feature')).toEqual(undefined)
         expect(window.console.warn).toHaveBeenCalledWith(
+            '[PostHog.js]',
             'getFeatureFlag for key "beta-feature" failed. Feature flags didn\'t load in time.'
         )
     })

diff --git a/src/__tests__/gdpr-utils.js b/src/__tests__/gdpr-utils.js
@@ -469,138 +469,4 @@ describe(`GDPR utils`, () => {
             })
         })
     })
-
-    describe(`addOptOutCheckPostHogLib`, () => {
-        const captureEventName = `єνєηт`
-        const captureProperties = { '𝖕𝖗𝖔𝖕𝖊𝖗𝖙𝖞': `𝓿𝓪𝓵𝓾𝓮` }
-        let capture, postHogLib
-
-        function setupMocks(config, silenceErrors = false) {
-            capture = sinon.spy()
-            postHogLib = {
-                config,
-                capture: undefined,
-            }
-            postHogLib.capture = gdpr.addOptOutCheck(postHogLib, capture, silenceErrors)
-        }
-
-        forPersistenceTypes(function (persistenceType) {
-            it(`should call the wrapped method if the user is neither opted in or opted out`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token, opt_out_capturing_persistence_type: persistenceType })
-
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.calledOnceWith(captureEventName, captureProperties)).toBe(true)
-                })
-            })
-
-            it(`should call the wrapped method if the user is opted in`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token, opt_out_capturing_persistence_type: persistenceType })
-
-                    gdpr.optIn(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.calledOnceWith(captureEventName, captureProperties)).toBe(true)
-                })
-            })
-
-            it(`should not call the wrapped method if the user is opted out`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token, opt_out_capturing_persistence_type: persistenceType })
-
-                    gdpr.optOut(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.notCalled).toBe(true)
-                })
-            })
-
-            it(`should not invoke the callback directly if the user is neither opted in or opted out`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token, opt_out_capturing_persistence_type: persistenceType })
-                    const callback = sinon.spy()
-
-                    postHogLib.capture(captureEventName, captureProperties, callback)
-
-                    expect(callback.notCalled).toBe(true)
-                })
-            })
-
-            it(`should not invoke the callback directly if the user is opted in`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token, opt_out_capturing_persistence_type: persistenceType })
-                    const callback = sinon.spy()
-
-                    gdpr.optIn(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties, callback)
-
-                    expect(callback.notCalled).toBe(true)
-                })
-            })
-
-            it(`should invoke the callback directly if the user is opted out`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token, opt_out_capturing_persistence_type: persistenceType })
-                    const callback = sinon.spy()
-
-                    gdpr.optOut(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties, callback)
-
-                    expect(callback.calledOnceWith(0)).toBe(true)
-                })
-            })
-
-            it(`should call the wrapped method if there is no token available`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({ token: null, opt_out_capturing_persistence_type: persistenceType })
-
-                    gdpr.optIn(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.calledOnceWith(captureEventName, captureProperties)).toBe(true)
-                })
-            })
-
-            it(`should call the wrapped method if config is undefined`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks(undefined, false)
-                    console.error = jest.fn()
-
-                    gdpr.optIn(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.calledOnceWith(captureEventName, captureProperties)).toBe(true)
-                    // :KLUDGE: Exact error message may vary between runtimes
-                    expect(console.error).toHaveBeenCalled()
-                })
-            })
-
-            it(`should allow use of a custom "persistence prefix" string`, () => {
-                TOKENS.forEach((token) => {
-                    setupMocks({
-                        token,
-                        opt_out_capturing_persistence_type: persistenceType,
-                        opt_out_capturing_cookie_prefix: CUSTOM_PERSISTENCE_PREFIX,
-                    })
-
-                    gdpr.optOut(token, { persistenceType, persistencePrefix: CUSTOM_PERSISTENCE_PREFIX })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.notCalled).toBe(true)
-
-                    gdpr.optIn(token, { persistenceType })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.notCalled).toBe(true)
-
-                    gdpr.optIn(token, { persistenceType, persistencePrefix: CUSTOM_PERSISTENCE_PREFIX })
-                    postHogLib.capture(captureEventName, captureProperties)
-
-                    expect(capture.calledOnceWith(captureEventName, captureProperties)).toBe(true)
-                })
-            })
-        })
-    })
 })
diff --git a/src/__tests__/posthog-core.identify.js b/src/__tests__/posthog-core.identify.js
@@ -225,11 +225,15 @@ describe('identify()', () => {
         it('does not update user', () => {
             console.error = jest.fn()
 
+            given.lib.debug()
             given.subject()
 
             expect(given.overrides.capture).not.toHaveBeenCalled()
             expect(given.overrides.register).not.toHaveBeenCalled()
-            expect(console.error).toHaveBeenCalledWith('Unique user id has not been set in posthog.identify')
+            expect(console.error).toHaveBeenCalledWith(
+                '[PostHog.js]',
+                'Unique user id has not been set in posthog.identify'
+            )
         })
     })
 

diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js
@@ -19,6 +19,7 @@ jest.mock('../utils', () => ({
 given('lib', () => {
     const posthog = new PostHog()
     posthog._init('testtoken', given.config, 'testhog')
+    posthog.debug()
     return Object.assign(posthog, given.overrides)
 })
 
@@ -111,7 +112,7 @@ describe('capture()', () => {
 
         expect(() => given.subject()).not.toThrow()
         expect(hook).not.toHaveBeenCalled()
-        expect(console.error).toHaveBeenCalledWith('No event name provided to posthog.capture')
+        expect(console.error).toHaveBeenCalledWith('[PostHog.js]', 'No event name provided to posthog.capture')
     })
 
     it('errors with object event name', () => {
@@ -123,7 +124,7 @@ describe('capture()', () => {
 
         expect(() => given.subject()).not.toThrow()
         expect(hook).not.toHaveBeenCalled()
-        expect(console.error).toHaveBeenCalledWith('No event name provided to posthog.capture')
+        expect(console.error).toHaveBeenCalledWith('[PostHog.js]', 'No event name provided to posthog.capture')
     })
 
     it('truncates long properties', () => {
@@ -509,6 +510,7 @@ describe('bootstrapping feature flags', () => {
         expect(given.lib.get_distinct_id()).not.toEqual(undefined)
         expect(given.lib.getFeatureFlag('multivariant')).toBe(undefined)
         expect(console.warn).toHaveBeenCalledWith(
+            '[PostHog.js]',
             expect.stringContaining('getFeatureFlag for key "multivariant" failed')
         )
         expect(given.lib.getFeatureFlag('disabled')).toBe(undefined)
@@ -861,6 +863,7 @@ describe('group()', () => {
 
         it('handles blank keys being passed', () => {
             window.console.error = jest.fn()
+            window.console.warn = jest.fn()
 
             given.lib.group(null, 'foo')
             given.lib.group('organization', null)
@@ -931,7 +934,7 @@ describe('_loaded()', () => {
 
         given.subject()
 
-        expect(console.error).toHaveBeenCalledWith('`loaded` function failed', expect.anything())
+        expect(console.error).toHaveBeenCalledWith('[PostHog.js]', '`loaded` function failed', expect.anything())
     })
 
     describe('/decide', () => {

diff --git a/src/__tests__/sessionid.js b/src/__tests__/sessionid.js
@@ -272,6 +272,7 @@ describe('Session ID manager', () => {
         })
 
         it('uses the custom session_idle_timeout_seconds if within bounds', () => {
+            window.POSTHOG_DEBUG = true
             expect(mockSessionManager(61)._sessionTimeoutMs).toEqual(61 * 1000)
             expect(console.warn).toBeCalledTimes(0)
             expect(mockSessionManager(59)._sessionTimeoutMs).toEqual(60 * 1000)

diff --git a/src/__tests__/setup.js b/src/__tests__/setup.js
@@ -1,8 +1,8 @@
 beforeEach(() => {
-    console.error = (message) => {
-        throw new Error(`Unexpected console.error: ${message}`)
+    console.error = (...args) => {
+        throw new Error(`Unexpected console.error: ${args}`)
     }
-    console.warn = (message) => {
-        throw new Error(`Unexpected console.warn: ${message}`)
+    console.warn = (...args) => {
+        throw new Error(`Unexpected console.warn: ${args}`)
     }
 })
diff --git a/src/autocapture-utils.ts b/src/autocapture-utils.ts
@@ -4,7 +4,7 @@
  * @returns {string} the element's class
  */
 import { AutocaptureConfig } from 'types'
-import { _each, _includes, _isUndefined, _trim } from './utils'
+import { _each, _includes, _isUndefined, _trim, logger } from './utils'
 
 export function getClassName(el: Element): string {
     switch (typeof el.className) {
@@ -335,7 +335,7 @@ export function getNestedSpanText(target: Element): string {
                         text = `${text} ${getNestedSpanText(child)}`.trim()
                     }
                 } catch (e) {
-                    console.error(e)
+                    logger.error(e)
                 }
             }
         })

diff --git a/src/autocapture.ts b/src/autocapture.ts
@@ -311,7 +311,7 @@ const autocapture = {
     afterDecideResponse: function (response: DecideResponse, instance: PostHog): void {
         const token = instance.config.token
         if (this._initializedTokens.indexOf(token) > -1) {
-            logger.log('autocapture already initialized for token "' + token + '"')
+            logger.info('autocapture already initialized for token "' + token + '"')
             return
         }