Skip to content

fix: skip profile reset when setProfile called with same identifiers#543

Open
jason-myers-klaviyo wants to merge 6 commits intomasterfrom
fix/push-token-request-storm
Open

fix: skip profile reset when setProfile called with same identifiers#543
jason-myers-klaviyo wants to merge 6 commits intomasterfrom
fix/push-token-request-storm

Conversation

@jason-myers-klaviyo
Copy link
Copy Markdown

Summary

Fixes excessive POST /client/push-tokens requests caused by setProfile() being called repeatedly with the same identifiers.

Root cause: enqueueProfile unconditionally called state.reset(preserveTokenData: false) regardless of whether the incoming identifiers matched current state. Each reset regenerated the anonymous ID (in reset() line 223), which triggered spurious push-token API requests. Under rate limiting, this compounded into a request storm.

Same bug as the Android SDK: klaviyo/klaviyo-android-sdk#435

Fix: Compare incoming (non-nil) identifiers against current state before calling reset(). Anonymous ID is the lowest-order identifier — there's no reason to regenerate it when higher-order identifiers (email, phone, externalId) haven't changed. resetProfile() remains available for explicitly clobbering all state.

Only non-nil identifiers are compared, since nil means "not specified" per updateStateWithProfile semantics.

Slack thread: https://klaviyo.slack.com/archives/C09DKBLQ3PX/p1775509483748149

Test plan

  • Verify unit tests pass in CI
  • setProfile(same identifiers) repeatedly — confirm no spurious push-token requests
  • setProfile(different identifiers) — confirm reset still fires, anonymous ID regenerated
  • setProfile(same identifiers, different attributes) — confirm attributes still update
  • resetProfile() — confirm still clobbers all state as before

🤖 Generated with Claude Code

When setProfile() was called with a profile whose identifiers matched
current state, it unconditionally called reset(), regenerating the
anonymous ID. This caused unnecessary anonymous ID churn, triggering
spurious push-token API requests — the same bug as the Android SDK
(klaviyo/klaviyo-android-sdk#435).

Now enqueueProfile only resets when a provided (non-nil) identifier
actually differs from current state. Anonymous ID is the lowest-order
identifier and doesn't need regeneration when higher-order identifiers
are unchanged. resetProfile() remains available for explicitly
clobbering all state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@njparadis njparadis left a comment

Choose a reason for hiding this comment

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

reviews for the review god, comments for the comment throne

evan-masseau and others added 2 commits April 9, 2026 11:26
…rm fix)

Cover the new identifiers-changed guard in enqueueProfile:
- Same identifiers → no reset, anonymousId preserved, pushTokenData stays
- Different identifiers → reset fires, pushTokenData cleared
- Same identifiers + different attributes → no reset, attributes still sent
- Partial identifier match → reset fires
- Nil identifiers (not specified) → skips comparison, no reset
- resetProfile → still clobbers all state unconditionally

Part of MAGE-473

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Profile values are trimmed (via trimWhiteSpaceOrReturnNilIfEmpty) before
being stored in state, but the identifiers-changed comparison was using
raw values. Whitespace-padded inputs like "  user@email.com  " would
never match the trimmed state value, defeating the optimization and
reproducing the original reset storm.

Apply the same normalization in the comparison. nil still means "not
specified" (skip comparison), but empty strings still trigger reset
(they represent explicit identifier clearing).

Part of MAGE-473

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The nil-skip approach (nil = "not specified", skip comparison) silently
changed setProfile from a clobbering to a merging operation for
unspecified fields. This is a bigger behavioral departure than intended
for this hotfix.

Align with Android: compare all three identifiers including nil, with
an isIdentified guard so fresh state (no prior identifiers) skips the
comparison. The only new behavior vs the old code is: exact match on
all identifiers → skip reset.

Part of MAGE-473

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ac611dc. Configure here.

With the isIdentified guard, setProfile on fresh state (no prior
identifiers) no longer triggers reset(). Two existing tests expected
pushTokenData to be cleared by reset — update them to reflect that
pushTokenData is now preserved when going from anonymous to identified.

Part of MAGE-473

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@evan-masseau evan-masseau requested a review from ndurell April 10, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants