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

fix: logging pointless error when offline #866

Merged
merged 3 commits into from
Oct 30, 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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { PostHog } from '../../../posthog-core'
import { DecideResponse, PostHogConfig } from '../../../types'
import { ExceptionObserver } from '../../../extensions/exception-autocapture'
import { window } from '../../../utils'
import { window } from '../../../utils/globals'

describe('Exception Observer', () => {
let exceptionObserver: ExceptionObserver
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/page-view.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { PageViewManager } from '../page-view'

const mockWindowGetter = jest.fn()
jest.mock('../utils', () => ({
...jest.requireActual('../utils'),
jest.mock('../utils/globals', () => ({
...jest.requireActual('../utils/globals'),
get window() {
return mockWindowGetter()
},
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/posthog-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { init_as_module, PostHog } from '../posthog-core'
import { PostHogPersistence } from '../posthog-persistence'
import { Decide } from '../decide'
import { autocapture } from '../autocapture'
import { window, document } from '../utils'

import { truth } from './helpers/truth'
import { _info } from '../utils/event-utils'
import { document, window } from '../utils/globals'

jest.mock('../gdpr-utils', () => ({
...jest.requireActual('../gdpr-utils'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { window } from '../../src/utils/globals'
import { RateLimiter } from '../rate-limiter'
import { logger } from '../utils/logger'

jest.mock('../../src/utils/logger')

describe('Rate Limiter', () => {
let rateLimiter: RateLimiter

beforeEach(() => {
jest.useFakeTimers()
rateLimiter = new RateLimiter()
jest.spyOn(window.console, 'error').mockImplementation()
})

it('is not rate limited with no batch key', () => {
Expand Down Expand Up @@ -110,4 +115,12 @@ describe('Rate Limiter', () => {

expect(rateLimiter.limits).toStrictEqual({})
})

it('does not log an error when there is an empty body', () => {
rateLimiter.checkForLimiting({
responseText: '',
})

expect(jest.mocked(logger).error).not.toHaveBeenCalled()
})
})
3 changes: 2 additions & 1 deletion src/autocapture-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
* @returns {string} the element's class
*/
import { AutocaptureConfig } from 'types'
import { _each, _includes, _trim, logger } from './utils'
import { _each, _includes, _trim } from './utils'

import { _isNull, _isString, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

export function getClassName(el: Element): string {
switch (typeof el.className) {
Expand Down
11 changes: 2 additions & 9 deletions src/autocapture.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
import {
_bind_instance_methods,
_each,
_extend,
_includes,
_register_event,
_safewrap_instance_methods,
logger,
} from './utils'
import { _bind_instance_methods, _each, _extend, _includes, _register_event, _safewrap_instance_methods } from './utils'
import {
getClassName,
getSafeText,
Expand All @@ -28,6 +20,7 @@ import { PostHog } from './posthog-core'
import { AUTOCAPTURE_DISABLED_SERVER_SIDE } from './constants'

import { _isBoolean, _isFunction, _isNull, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

function limitText(length: number, text: string): string {
if (text.length > length) {
Expand Down
3 changes: 2 additions & 1 deletion src/decide.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { autocapture } from './autocapture'
import { _base64Encode, loadScript, logger } from './utils'
import { _base64Encode, loadScript } from './utils'
import { PostHog } from './posthog-core'
import { DecideResponse } from './types'
import { STORED_GROUP_PROPERTIES_KEY, STORED_PERSON_PROPERTIES_KEY } from './constants'

import { _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

export class Decide {
instance: PostHog
Expand Down
3 changes: 2 additions & 1 deletion src/extensions/exception-autocapture/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { logger, window } from '../../utils'
import { window } from '../../utils/globals'
import { PostHog } from '../../posthog-core'
import { DecideResponse, Properties } from '../../types'
import { ErrorEventArgs, ErrorProperties, errorToProperties, unhandledRejectionToProperties } from './error-conversion'
import { isPrimitive } from './type-checking'

import { _isArray, _isObject, _isUndefined } from '../../utils/type-utils'
import { logger } from '../../utils/logger'

const EXCEPTION_INGESTION_ENDPOINT = '/e/'

Expand Down
3 changes: 2 additions & 1 deletion src/extensions/replay/sessionrecording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import { PostHog } from '../../posthog-core'
import { DecideResponse, NetworkRequest, Properties } from '../../types'
import { EventType, type eventWithTime, type listenerHandler } from '@rrweb/types'
import Config from '../../config'
import { _timestamp, loadScript, logger } from '../../utils'
import { _timestamp, loadScript } from '../../utils'

import { _isBoolean, _isNull, _isNumber, _isObject, _isString, _isUndefined } from '../../utils/type-utils'
import { logger } from '../../utils/logger'

const BASE_ENDPOINT = '/s/'

Expand Down
2 changes: 1 addition & 1 deletion src/extensions/replay/web-performance.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { logger } from '../../utils'
import { PostHog } from '../../posthog-core'
import { DecideResponse, NetworkRequest } from '../../types'
import { isLocalhost } from '../../utils/request-utils'

import { _isUndefined } from '../../utils/type-utils'
import { logger } from '../../utils/logger'

const PERFORMANCE_EVENTS_MAPPING: { [key: string]: number } = {
// BASE_PERFORMANCE_EVENT_COLUMNS
Expand Down
2 changes: 1 addition & 1 deletion src/extensions/segment-integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* })
* ```
*/
import { logger } from '../utils'
import { PostHog } from '../posthog-core'
import { logger } from '../utils/logger'

// Loosely based on https://github.com/segmentio/analytics-next/blob/master/packages/core/src/plugins/index.ts
interface SegmentPluginContext {
Expand Down
4 changes: 3 additions & 1 deletion src/extensions/toolbar.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { _register_event, _try, loadScript, logger, window } from '../utils'
import { _register_event, _try, loadScript } from '../utils'
import { PostHog } from '../posthog-core'
import { DecideResponse, ToolbarParams } from '../types'
import { POSTHOG_MANAGED_HOSTS } from './cloud'
import { _getHashParam } from '../utils/request-utils'
import { logger } from '../utils/logger'
import { window } from '../utils/globals'

// TRICKY: Many web frameworks will modify the route on load, potentially before posthog is initialized.
// To get ahead of this we grab it as soon as the posthog-js is parsed
Expand Down
4 changes: 3 additions & 1 deletion src/gdpr-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
* These functions are used internally by the SDK and are not intended to be publicly exposed.
*/

import { _each, _includes, logger, window } from './utils'
import { _each, _includes } from './utils'
import { window } from './utils/globals'
import { cookieStore, localStore, localPlusCookieStore } from './storage'
import { GDPROptions, PersistentStore } from './types'
import { PostHog } from './posthog-core'

import { _isNumber, _isString } from './utils/type-utils'
import { logger } from './utils/logger'

/**
* A function used to capture a PostHog event (e.g. PostHogLib.capture)
Expand Down
2 changes: 1 addition & 1 deletion src/page-view.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { window } from './utils'
import { window } from './utils/globals'

interface PageViewData {
pathname: string
Expand Down
7 changes: 3 additions & 4 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import {
_isBlockedUA,
_register_event,
_safewrap_class,
userAgent,
window,
logger,
isCrossDomainCookie,
} from './utils'
import { window } from './utils/globals'
import { autocapture } from './autocapture'
import { PostHogFeatureFlags } from './posthog-featureflags'
import { PostHogPersistence } from './posthog-persistence'
Expand Down Expand Up @@ -55,9 +53,10 @@ import { PostHogSurveys } from './posthog-surveys'
import { RateLimiter } from './rate-limiter'
import { uuidv7 } from './uuidv7'
import { SurveyCallback } from './posthog-surveys-types'
import { document } from './utils'
import { _isArray, _isEmptyObject, _isFunction, _isObject, _isString, _isUndefined } from './utils/type-utils'
import { _info } from './utils/event-utils'
import { logger } from './utils/logger'
import { document, userAgent } from './utils/globals'

/*
SIMPLE STYLE GUIDE:
Expand Down
3 changes: 2 additions & 1 deletion src/posthog-featureflags.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { _base64Encode, _entries, _extend, logger } from './utils'
import { _base64Encode, _entries, _extend } from './utils'
import { PostHog } from './posthog-core'
import {
DecideResponse,
Expand All @@ -20,6 +20,7 @@ import {
} from './constants'

import { _isArray } from './utils/type-utils'
import { logger } from './utils/logger'

const PERSISTENCE_ACTIVE_FEATURE_FLAGS = '$active_feature_flags'
const PERSISTENCE_OVERRIDE_FEATURE_FLAGS = '$override_feature_flags'
Expand Down
3 changes: 2 additions & 1 deletion src/posthog-persistence.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint camelcase: "off" */

import { _each, _extend, _include, _strip_empty_properties, logger } from './utils'
import { _each, _extend, _include, _strip_empty_properties } from './utils'
import { cookieStore, localStore, localPlusCookieStore, memoryStore, sessionStore } from './storage'
import { PersistentStore, PostHogConfig, Properties } from './types'
import {
Expand All @@ -13,6 +13,7 @@ import {

import { _isObject, _isUndefined } from './utils/type-utils'
import { _info } from './utils/event-utils'
import { logger } from './utils/logger'

const CASE_INSENSITIVE_PERSISTENCE_TYPES: readonly Lowercase<PostHogConfig['persistence']>[] = [
'cookie',
Expand Down
9 changes: 7 additions & 2 deletions src/rate-limiter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { logger } from './utils'
import { logger } from './utils/logger'

const oneMinuteInMilliseconds = 60 * 1000

Expand All @@ -20,7 +20,12 @@ export class RateLimiter {

public checkForLimiting = (xmlHttpRequest: XMLHttpRequest): void => {
try {
const response: CaptureResponse = JSON.parse(xmlHttpRequest.responseText)
const text = xmlHttpRequest.responseText
if (!text || !text.length) {
return
}

const response: CaptureResponse = JSON.parse(text)
const quotaLimitedProducts = response.quota_limited || []
Comment on lines +23 to +28
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the actual fix (ignoring the noise from refactoring logger into its own file to make it easier to mock the smallest area in the test)

quotaLimitedProducts.forEach((batchKey) => {
logger.info(`[RateLimiter] ${batchKey || 'events'} is quota limited.`)
Expand Down
2 changes: 1 addition & 1 deletion src/retry-queue.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { RequestQueueScaffold } from './base-request-queue'
import { encodePostData, xhr } from './send-request'
import { QueuedRequestData, RetryQueueElement } from './types'
import { logger } from './utils'
import { RateLimiter } from './rate-limiter'

import { _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

const thirtyMinutes = 30 * 60 * 1000

Expand Down
3 changes: 2 additions & 1 deletion src/send-request.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { _each, logger } from './utils'
import { _each } from './utils'
import Config from './config'
import { PostData, XHROptions, XHRParams } from './types'
import { _HTTPBuildQuery } from './utils/request-utils'

import { _isArray, _isFunction, _isNumber, _isUint8Array, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

export const addParamsToURL = (
url: string,
Expand Down
3 changes: 2 additions & 1 deletion src/sessionid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import { SESSION_ID } from './constants'
import { sessionStore } from './storage'
import { PostHogConfig, SessionIdChangedCallback } from './types'
import { uuidv7 } from './uuidv7'
import { logger, window } from './utils'
import { window } from './utils/globals'

import { _isArray, _isNumber, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

const MAX_SESSION_IDLE_TIMEOUT = 30 * 60 // 30 minutes
const MIN_SESSION_IDLE_TIMEOUT = 60 // 1 minute
Expand Down
3 changes: 2 additions & 1 deletion src/storage.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { _extend, logger } from './utils'
import { _extend } from './utils'
import { PersistentStore, Properties } from './types'
import { DISTINCT_ID, SESSION_ID, SESSION_RECORDING_IS_SAMPLED } from './constants'

import { _isNull, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

const DOMAIN_MATCH_REGEX = /[a-z0-9][a-z0-9-]+\.[a-z]{2,}$/i

Expand Down
3 changes: 2 additions & 1 deletion src/utils/event-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { _getQueryParam } from './request-utils'
import { _isNull, _isUndefined } from './type-utils'
import { Properties } from '../types'
import Config from '../config'
import { _each, _extend, _includes, _strip_empty_properties, _timestamp, document, userAgent } from './index'
import { _each, _extend, _includes, _strip_empty_properties, _timestamp } from './index'
import { document, userAgent } from './globals'

export const _info = {
campaignParams: function (customParams?: string[]): Record<string, any> {
Expand Down
12 changes: 12 additions & 0 deletions src/utils/globals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Saved references to long variable names, so that bundling can minimize file size.
*/
export const ArrayProto = Array.prototype
export const nativeForEach = ArrayProto.forEach
export const nativeIndexOf = ArrayProto.indexOf
export const win: Window & typeof globalThis = typeof window !== 'undefined' ? window : ({} as typeof window)
const navigator = win.navigator || { userAgent: '' }
const document = win.document || {}
const userAgent = navigator.userAgent

export { win as window, userAgent, document }
Loading
Loading