Skip to content

[SDK-369] no push reregister on token refresh#1003

Open
franco-zalamena-iterable wants to merge 11 commits intomasterfrom
bugfix/no-push-reregister-on-token-refresh
Open

[SDK-369] no push reregister on token refresh#1003
franco-zalamena-iterable wants to merge 11 commits intomasterfrom
bugfix/no-push-reregister-on-token-refresh

Conversation

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor

@franco-zalamena-iterable franco-zalamena-iterable commented Mar 13, 2026

🔹 Jira Ticket(s) if any

✏️ Description

Fixed a problem with completeuserlogin retrigger

Decoupling the completeUserLogin from the SetAuthToken flow. Those live independently now.

Summary

setAuthToken replaced with updateAuthToken. This only handles the update of the auth token without the side effects of login. If the user expects a login flow, the setEmail should be called. This prevents unnecessary calls to the backend on token refreshes

@franco-zalamena-iterable franco-zalamena-iterable force-pushed the bugfix/no-push-reregister-on-token-refresh branch from 68f10bc to 6bc8f7b Compare March 13, 2026 14:29
@franco-zalamena-iterable franco-zalamena-iterable force-pushed the bugfix/no-push-reregister-on-token-refresh branch from db3f49f to bc065e5 Compare March 17, 2026 10:40
@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

Wait for bugbash to be completed

Base automatically changed from feature/auto-retry to autoretry-master March 19, 2026 10:43
Base automatically changed from autoretry-master to master March 23, 2026 21:52
@franco-zalamena-iterable franco-zalamena-iterable changed the title Bugfix/no push reregister on token refresh [SDK-406] no push reregister on token refresh Apr 1, 2026
@franco-zalamena-iterable franco-zalamena-iterable force-pushed the bugfix/no-push-reregister-on-token-refresh branch from bc065e5 to 6d3a0b7 Compare April 2, 2026 08:14
@franco-zalamena-iterable franco-zalamena-iterable changed the title [SDK-406] no push reregister on token refresh [SDK-386] no push reregister on token refresh Apr 8, 2026
@franco-zalamena-iterable franco-zalamena-iterable changed the title [SDK-386] no push reregister on token refresh [SDK-369] no push reregister on token refresh Apr 8, 2026
The deprecated setAuthToken() was unconditionally calling
completeUserLogin() via storeAuthData(). On master, it only triggered
login when the token actually changed. Restore that guard to prevent
unnecessary login cycles when callers pass the same token.

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@sumeruchat sumeruchat left a comment

Choose a reason for hiding this comment

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

AI Review

  1. Behavior change for setEmail(sameEmail) is a compat break. Apps that were calling it repeatedly as an idempotent no-op will now see extra syncs and backend
    calls. I don't think it's in the CHANGELOG — worth mentioning there since it's an observable runtime change.
  2. Deprecation warning log fires on every setAuthToken call. For apps that haven't migrated, that's potentially thousands of log lines per session. Consider
    IterableLogger.w once-per-session (static AtomicBoolean).
  3. updateAuthToken(null) silently stores null with no validation. Previously the old setAuthToken(null, true) path went through completeUserLogin which
    rejected null under JWT. This is fine given the split, but means updateAuthToken is a footgun — a caller on the public API could clear the token in a state
    where the SDK then makes unauthenticated requests. Maybe worth a @nonnull or at least no-op on null.
  4. Bugbash gate: since Franco asked to wait for bugbash, probably worth approving conditionally or holding review approval until QA signs off — especially
    given #1 is a behavioral change.

My review

This look a little bit dangerous to me as its behavior change and since no one is asking for this do we need to do this? @franco-zalamena-iterable Basically i see only potential risks of breaking existing behaviour with no upside.

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.

5 participants