Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Move all logs everything over to logger #830

Merged
merged 6 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const rules = {
'@typescript-eslint/no-unused-vars': ['error'],
'no-prototype-builtins': 'off',
'no-empty': 'off',
'no-console': 'error',
}

const extend = [
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions react/src/context/PostHogProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import posthogJs, { PostHogConfig } from 'posthog-js'

import React, { useMemo } from 'react'
Expand All @@ -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.'
)
}

Expand All @@ -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)
}
Expand Down
6 changes: 4 additions & 2 deletions src/__tests__/decide.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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.'
)
})
})
Expand Down
3 changes: 3 additions & 0 deletions src/__tests__/featureflags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
)
})
Expand Down
134 changes: 0 additions & 134 deletions src/__tests__/gdpr-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
})
})
6 changes: 5 additions & 1 deletion src/__tests__/posthog-core.identify.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)
})
})

Expand Down
9 changes: 6 additions & 3 deletions src/__tests__/posthog-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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', () => {
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/sessionid.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions src/__tests__/setup.js
Original file line number Diff line number Diff line change
@@ -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}`)
}
})
4 changes: 2 additions & 2 deletions src/autocapture-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -335,7 +335,7 @@ export function getNestedSpanText(target: Element): string {
text = `${text} ${getNestedSpanText(child)}`.trim()
}
} catch (e) {
console.error(e)
logger.error(e)
}
}
})
Expand Down
2 changes: 1 addition & 1 deletion src/autocapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading
Loading