-
Notifications
You must be signed in to change notification settings - Fork 237
fix(identify): reject invalid values passed to identify #3285
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
base: main
Are you sure you want to change the base?
Changes from all commits
7bd15d0
c1adc03
1521ae0
d0930a5
3965042
334f867
f72c8d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| 'posthog-js': minor | ||
| '@posthog/types': minor | ||
| --- | ||
|
|
||
| Add strict_identify config option to throw on invalid distinct IDs in posthog.identify(). Defaults to true when using defaults >= '2026-04-01'. When disabled, invalid calls now log a console error instead of being silently ignored. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,7 @@ | |
| extend, | ||
| isCrossDomainCookie, | ||
| migrateConfigField, | ||
| PostHogConfigError, | ||
| safewrapClass, | ||
| } from './utils' | ||
| import { isLikelyBot } from './utils/blocked-uas' | ||
|
|
@@ -168,12 +169,14 @@ | |
| | 'session_recording' | ||
| | 'external_scripts_inject_target' | ||
| | 'internal_or_test_user_hostname' | ||
| | 'strict_identify' | ||
| > => ({ | ||
| rageclick: defaults && defaults >= '2025-11-30' ? { content_ignorelist: true } : true, | ||
| capture_pageview: defaults && defaults >= '2025-05-24' ? 'history_change' : true, | ||
| session_recording: defaults && defaults >= '2025-11-30' ? { strictMinimumDuration: true } : {}, | ||
| external_scripts_inject_target: defaults && defaults >= '2026-01-30' ? 'head' : 'body', | ||
| internal_or_test_user_hostname: defaults && defaults >= '2026-01-30' ? /^(localhost|127\.0\.0\.1)$/ : undefined, | ||
| strict_identify: defaults && defaults >= '2026-04-01' ? true : false, | ||
| }) | ||
|
|
||
| // NOTE: Remember to update `types.ts` when changing a default value | ||
|
|
@@ -2280,6 +2283,31 @@ | |
| ) | ||
| } | ||
|
|
||
| private _validateIdentifyId(id: string | undefined): string | undefined { | ||
| if (!id) { | ||
| return 'Unique user id has not been set in posthog.identify' | ||
| } | ||
| let reason: string | undefined | ||
| if (id === COOKIELESS_SENTINEL_VALUE) { | ||
| reason = 'This ID is only used as a sentinel value.' | ||
| } else if ( | ||
| isDistinctIdStringLike(id) || | ||
| (this.config.strict_identify && ['undefined', 'null'].includes(id.toLowerCase())) | ||
| ) { | ||
| reason = 'This ID should be unique to the user and not a hardcoded string.' | ||
| } | ||
| return reason | ||
| ? `The string "${id}" was set in posthog.identify which indicates an error. ${reason}` | ||
| : undefined | ||
|
Comment on lines
+2299
to
+2301
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This multi-line ternary expression with template literal needs to be reformatted to match Prettier's style rules. Either put it on a single line or restructure the logic to avoid the formatting conflict. Spotted by Graphite (based on CI logs) |
||
| } | ||
|
|
||
| private _identifyError(message: string): void { | ||
| if (this.config.strict_identify) { | ||
| throw new PostHogConfigError('[PostHog.js] ' + message) | ||
| } | ||
| logger.critical(message) | ||
| } | ||
|
Check failure on line 2309 in packages/browser/src/posthog-core.ts
|
||
|
|
||
| /** | ||
| * Associates a user with a unique identifier instead of an auto-generated ID. | ||
| * Learn more about [identifying users](/docs/product-analytics/identify) | ||
|
|
@@ -2332,22 +2360,9 @@ | |
| ) | ||
| } | ||
|
|
||
| //if the new_distinct_id has not been set ignore the identify event | ||
| if (!new_distinct_id) { | ||
| logger.error('Unique user id has not been set in posthog.identify') | ||
| return | ||
| } | ||
|
|
||
| if (isDistinctIdStringLike(new_distinct_id)) { | ||
| logger.critical( | ||
| `The string "${new_distinct_id}" was set in posthog.identify which indicates an error. This ID should be unique to the user and not a hardcoded string.` | ||
| ) | ||
| return | ||
| } | ||
| if (new_distinct_id === COOKIELESS_SENTINEL_VALUE) { | ||
| logger.critical( | ||
| `The string "${COOKIELESS_SENTINEL_VALUE}" was set in posthog.identify which indicates an error. This ID is only used as a sentinel value.` | ||
| ) | ||
| const identifyError = this._validateIdentifyId(new_distinct_id) | ||
| if (identifyError || !new_distinct_id) { | ||
| this._identifyError(identifyError ?? 'Unique user id has not been set in posthog.identify') | ||
| return | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -284,7 +284,7 @@ export interface HeatmapConfig { | |
| flush_interval_milliseconds: number | ||
| } | ||
|
|
||
| export type ConfigDefaults = '2026-01-30' | '2025-11-30' | '2025-05-24' | 'unset' | ||
| export type ConfigDefaults = '2026-04-01' | '2026-01-30' | '2025-11-30' | '2025-05-24' | 'unset' | ||
|
|
||
| export type ExternalIntegrationKind = 'intercom' | 'crispChat' | ||
|
|
||
|
|
@@ -1014,11 +1014,21 @@ export interface PostHogConfig { | |
| * - `'2025-05-24'`: Use updated default behaviors (e.g. capture_pageview defaults to 'history_change') | ||
| * - `'2025-11-30'`: Defaults from '2025-05-24' plus additional changes (e.g. strict minimum duration for replay and rageclick content ignore list defaults to active) | ||
| * - `'2026-01-30'`: Defaults from '2025-11-30' plus external_scripts_inject_target defaults to 'head' (avoids SSR hydration errors) | ||
| * - `'2026-04-01'`: Defaults from '2026-01-30' plus strict_identify defaults to true (throws on invalid distinct IDs) | ||
| * | ||
| * @default 'unset' | ||
| */ | ||
| defaults: ConfigDefaults | ||
|
|
||
| /** | ||
| * When true, posthog.identify() throws an error if called with an invalid distinct_id | ||
| * (e.g. undefined, null, empty string, or the string "undefined"). | ||
| * When false, invalid calls log a console error and return without identifying. | ||
|
Comment on lines
+1024
to
+1026
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think SDKs should never throw, swallow users input and log is the best you can do to be a good citizen
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, fair... i was being overly keen |
||
| * | ||
| * @default false (or true when defaults >= '2026-04-01') | ||
| */ | ||
| strict_identify: boolean | ||
|
|
||
| /** | ||
| * EXPERIMENTAL: Defers initialization of non-critical extensions (autocapture, session recording, etc.) | ||
| * to the next event loop tick using setTimeout. This reduces main thread blocking during SDK | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.