Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 7. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughVersion 2.5.4 introduces CleverTap custom ID support in the On User Login flow. Changes include custom ID validation functions, integration into OUL logic with conditional branching, a new info-level logging capability, logging level adjustments for null/empty value scenarios, and corresponding tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant OUL as On User Login Module
participant Validator as Custom ID Validator
participant Logger as Logger Module
participant Storage as Local Storage
Client->>OUL: Trigger OUL with profile (customId)
OUL->>Validator: validateCustomCleverTapID(customId)
alt Valid Custom ID
Validator->>Validator: Lowercase & sanitize
Validator->>Logger: Log success (info level)
Logger->>Logger: reportInfo with CLEVERTAP_INFO_PREFIX
Validator-->>OUL: {isValid: true, sanitizedId}
OUL->>Storage: Set gcookie = sanitizedId
OUL->>OUL: Set hasCustomId = true
OUL->>OUL: Bypass default gcookie assignment
else Invalid Custom ID
Validator->>Logger: Log error (error level)
Validator-->>OUL: {isValid: false, error}
OUL->>OUL: Set hasCustomId = false
OUL->>OUL: Continue with standard flow
end
OUL->>Client: Complete OUL processing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.5)clevertap.jsThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
CHANGELOG.md (1)
4-5: Trailing whitespace on these lines.Lines 4 and 5 have trailing spaces after the content. Minor nit but worth cleaning up for consistency with the rest of the file.
🧹 Remove trailing whitespace
-## [2.5.4] 19th Feb 2026 -- Added Clevertap Custom Id Support in On User Login. +## [2.5.4] 19th Feb 2026 +- Added Clevertap Custom Id Support in On User Login.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 4 - 5, Remove the trailing whitespace on the two lines shown in the diff: the header "## [2.5.4] 19th Feb 2026" and the bullet "- Added Clevertap Custom Id Support in On User Login." — open CHANGELOG.md and delete any trailing spaces at the end of those lines so they match the rest of the file formatting.src/modules/userLogin.js (1)
186-201: Truthy check onprofileObj.customIdsilently discards falsy non-null values without feedback.Line 186 uses
if (profileObj.customId)which is falsy for'',0,false, etc. These fall through to theelse ifon line 198, which deletescustomIdsilently — no validation error is logged. SincevalidateCustomCleverTapIDalready returns a clear error for non-string inputs (and empty strings fail the length check), consider routing all definedcustomIdvalues through the validator so users get feedback.♻️ Suggested change — validate all defined customId values
- if (profileObj.customId) { + if (profileObj.customId != null && profileObj.customId !== false) { const result = validateCustomCleverTapID(profileObj.customId) if (result.isValid) { hasCustomId = true @@ .. } delete profileObj.customId - } else if ('customId' in profileObj) { - // Key present but falsy (e.g. '', 0) — remove so it is not sent as a profile field - delete profileObj.customId }Or more simply, check for key presence:
- if (profileObj.customId) { + if ('customId' in profileObj) { const result = validateCustomCleverTapID(profileObj.customId)This way
'',0,null, etc. all go through validation and log the appropriate error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/userLogin.js` around lines 186 - 201, The current truthy check on profileObj.customId skips validation for falsy but defined values (e.g. '', 0, false); change the branch to detect presence of the key (use if ('customId' in profileObj) or if (Object.prototype.hasOwnProperty.call(profileObj, 'customId'))), then call validateCustomCleverTapID(profileObj.customId) for all defined values; on success set hasCustomId, this.#device.gcookie and StorageManager.saveToLSorCookie(GCOOKIE_NAME, result.sanitizedId) and log via this.#logger.debug, on failure log the validation error via this.#logger.error, and finally delete profileObj.customId so it is not sent as a profile field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clevertap.js`:
- Around line 9043-9047: The reportInfo method is incorrectly mutating the
shared error-tracking object this.wzrkError (same fields set by reportError)
which corrupts downstream telemetry; remove the two assignments this.wzrkError.c
and this.wzrkError.d from reportInfo and leave only the info logging call (keep
reportError unchanged so it still sets this.wzrkError), and ensure
request-building code in src/modules/request.js continues to rely on
this.wzrkError being modified only by reportError or true error paths.
- Around line 12307-12313: The storage write unconditionally overwrites the
custom ID when a cached user is found; modify the add-to-cache branch so
StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) is executed only when
customIdFlag is false (same guard that sets _classPrivateFieldLooseBase(this,
_device$2)[_device$2].gcookie). Locate the block that calls
$ct.LRU_CACHE.get(kId) and move the
StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) call inside the if
(!customIdFlag) conditional so cached restores don't clobber a provided
customId.
In `@src/modules/logger.js`:
- Around line 75-79: reportInfo currently mutates the shared wzrkError
(this.wzrkError.c/d) which overwrites earlier error codes set by reportError
during validation flows like isChargedEventStructureValid -> isObjStructureValid
-> cleanNullEmptyValues; change reportInfo to stop writing into this.wzrkError
and instead write into a separate informational object (e.g., this.wzrkInfo.c
and this.wzrkInfo.d) or only log to console if info should not be included in
payloads; update the reportInfo implementation to set this.wzrkInfo rather than
this.wzrkError and keep the existing info(...) call, and ensure any
request-building code continues to use this.wzrkError for error signaling so
error-level codes are not lost.
In `@src/modules/userLogin.js`:
- Around line 119-125: The storage write unconditionally overwrites a custom
gcookie: when retrieving gFromCache via $ct.LRU_CACHE.get(kId) you must not call
StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) if customIdFlag is
true; change the logic around StorageManager.saveToLSorCookie and the
this.#device.gcookie assignment so both the in-memory and persistent store are
only updated when customIdFlag is false (keep the existing guard on
this.#device.gcookie and add the same guard around
StorageManager.saveToLSorCookie).
In `@test/unit/clevertap.spec.js`:
- Around line 158-180: The tests currently only exercise the mocked
validateCustomCleverTapID; replace them with integration-style tests that
exercise the actual OUL flow: call onUserLogin.push(...) with a profile that
includes customId, let the flow run (mock network/store as needed), then assert
the real observable side effects — gcookie is assigned, storage write(s) occur
with the expected values, and the outgoing profile/request payload does not
include customId (i.e., customId is stripped) before the network request; use
the real validateCustomCleverTapID implementation rather than mocking it so the
full behavior is validated.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 4-5: Remove the trailing whitespace on the two lines shown in the
diff: the header "## [2.5.4] 19th Feb 2026" and the bullet "- Added Clevertap
Custom Id Support in On User Login." — open CHANGELOG.md and delete any trailing
spaces at the end of those lines so they match the rest of the file formatting.
In `@src/modules/userLogin.js`:
- Around line 186-201: The current truthy check on profileObj.customId skips
validation for falsy but defined values (e.g. '', 0, false); change the branch
to detect presence of the key (use if ('customId' in profileObj) or if
(Object.prototype.hasOwnProperty.call(profileObj, 'customId'))), then call
validateCustomCleverTapID(profileObj.customId) for all defined values; on
success set hasCustomId, this.#device.gcookie and
StorageManager.saveToLSorCookie(GCOOKIE_NAME, result.sanitizedId) and log via
this.#logger.debug, on failure log the validation error via this.#logger.error,
and finally delete profileObj.customId so it is not sent as a profile field.
| reportInfo(code, description) { | ||
| this.wzrkError.c = code; | ||
| this.wzrkError.d = description; | ||
| this.info("".concat(CLEVERTAP_INFO_PREFIX, " ").concat(code, ": ").concat(description)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'wzrkError' --type=js -C 3Repository: CleverTap/clevertap-web-sdk
Length of output: 3926
reportInfo overwrites wzrkError state intended for error tracking.
Both reportInfo and reportError mutate this.wzrkError.c and this.wzrkError.d identically. The wzrkError object is consumed downstream in request building (checked in src/modules/request.js) and added to telemetry payloads. When reportInfo is called, it overwrites any previously stored error state before wzrkError is serialized, silently corrupting error telemetry data.
Since info-level messages should not feed into error tracking, remove the wzrkError mutations from reportInfo:
Proposed fix
reportInfo(code, description) {
- this.wzrkError.c = code;
- this.wzrkError.d = description;
this.info("".concat(CLEVERTAP_INFO_PREFIX, " ").concat(code, ": ").concat(description));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clevertap.js` around lines 9043 - 9047, The reportInfo method is incorrectly
mutating the shared error-tracking object this.wzrkError (same fields set by
reportError) which corrupts downstream telemetry; remove the two assignments
this.wzrkError.c and this.wzrkError.d from reportInfo and leave only the info
logging call (keep reportError unchanged so it still sets this.wzrkError), and
ensure request-building code in src/modules/request.js continues to rely on
this.wzrkError being modified only by reportError or true error paths.
| const gFromCache = $ct.LRU_CACHE.get(kId); | ||
| $ct.LRU_CACHE.set(kId, gFromCache); | ||
| StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache); | ||
| _classPrivateFieldLooseBase(this, _device$2)[_device$2].gcookie = gFromCache; | ||
| StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache); // Only override gcookie if we don't have a customId | ||
|
|
||
| if (!customIdFlag) { | ||
| _classPrivateFieldLooseBase(_this, _device$2)[_device$2].gcookie = gFromCache; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the source files (not bundled clevertap.js)
find . -type f -name "*.js" | grep -E "(src|source|lib)" | head -20Repository: CleverTap/clevertap-web-sdk
Length of output: 622
🏁 Script executed:
# Search for the addToK function and customIdFlag usage
rg "addToK|customIdFlag" -t js -A 5 -B 5Repository: CleverTap/clevertap-web-sdk
Length of output: 5383
🏁 Script executed:
# Look for the specific code at the claimed lines in clevertap.js
rg "const gFromCache = \\\$ct\.LRU_CACHE\.get" -t js -A 10 -B 5Repository: CleverTap/clevertap-web-sdk
Length of output: 2058
🏁 Script executed:
# Find where custom ID is processed (hunk 9 referenced at lines 12393-12394)
rg "GCOOKIE_NAME|sanitizedId|customId" -t js -A 3 -B 3 | head -100Repository: CleverTap/clevertap-web-sdk
Length of output: 5686
🏁 Script executed:
# Find where custom ID is set (sanitized) and saved to storage in `#processOUL`
rg "sanitizedId|hasCustomId" src/modules/userLogin.js -B 5 -A 5Repository: CleverTap/clevertap-web-sdk
Length of output: 1726
🏁 Script executed:
# Get more context around the `#processOUL` function to understand the flow
cat -n src/modules/userLogin.js | head -200Repository: CleverTap/clevertap-web-sdk
Length of output: 7827
🏁 Script executed:
# Look at the complete addToK function and `#processOUL` context
sed -n '1,300p' src/modules/userLogin.js | cat -nRepository: CleverTap/clevertap-web-sdk
Length of output: 12265
Storage overwrite negates custom ID when returning user is in cache.
When customIdFlag is true, the custom ID is saved to GCOOKIE_NAME in storage at line 192. However, if the user is found in the LRU cache (line 110), line 121 unconditionally overwrites GCOOKIE_NAME with gFromCache, undoing the custom ID assignment. The guard on lines 123–125 only protects device.gcookie but not the storage write—creating an inconsistency with the non-cached user path (lines 137–139) where the storage write is properly guarded.
Scenario: User provides customId + user exists in cache → custom ID saved at line 192 → addToK retrieves cached GUID → line 121 overwrites storage with cached value → custom ID lost.
Move the storage write inside the conditional to match the device assignment guard:
const gFromCache = $ct.LRU_CACHE.get(kId)
$ct.LRU_CACHE.set(kId, gFromCache)
- StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache)
// Only override gcookie if we don't have a customId
if (!customIdFlag) {
+ StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache)
this.#device.gcookie = gFromCache
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const gFromCache = $ct.LRU_CACHE.get(kId); | |
| $ct.LRU_CACHE.set(kId, gFromCache); | |
| StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache); | |
| _classPrivateFieldLooseBase(this, _device$2)[_device$2].gcookie = gFromCache; | |
| StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache); // Only override gcookie if we don't have a customId | |
| if (!customIdFlag) { | |
| _classPrivateFieldLooseBase(_this, _device$2)[_device$2].gcookie = gFromCache; | |
| } | |
| const gFromCache = $ct.LRU_CACHE.get(kId); | |
| $ct.LRU_CACHE.set(kId, gFromCache); | |
| // Only override gcookie if we don't have a customId | |
| if (!customIdFlag) { | |
| StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache); | |
| _classPrivateFieldLooseBase(_this, _device$2)[_device$2].gcookie = gFromCache; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clevertap.js` around lines 12307 - 12313, The storage write unconditionally
overwrites the custom ID when a cached user is found; modify the add-to-cache
branch so StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) is executed
only when customIdFlag is false (same guard that sets
_classPrivateFieldLooseBase(this, _device$2)[_device$2].gcookie). Locate the
block that calls $ct.LRU_CACHE.get(kId) and move the
StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) call inside the if
(!customIdFlag) conditional so cached restores don't clobber a provided
customId.
| reportInfo (code, description) { | ||
| this.wzrkError.c = code | ||
| this.wzrkError.d = description | ||
| this.info(`${CLEVERTAP_INFO_PREFIX} ${code}: ${description}`) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for wzrkError usage across the codebase
rg -n 'wzrkError' --type=js -C 2Repository: CleverTap/clevertap-web-sdk
Length of output: 3163
🏁 Script executed:
#!/bin/bash
# Search for cleanNullEmptyValues and how it uses reportInfo
rg -n 'cleanNullEmptyValues' --type=js -C 5Repository: CleverTap/clevertap-web-sdk
Length of output: 5515
🏁 Script executed:
#!/bin/bash
# Search for reportInfo calls to understand how it's used
rg -n 'reportInfo' --type=js -C 3Repository: CleverTap/clevertap-web-sdk
Length of output: 4390
🏁 Script executed:
#!/bin/bash
# Search for reportError calls to understand when errors are set
rg -n 'reportError' --type=js -C 3Repository: CleverTap/clevertap-web-sdk
Length of output: 17573
🏁 Script executed:
#!/bin/bash
# Verify the exact flow in isObjStructureValid to confirm the overwrite sequence
sed -n '170,220p' src/util/validator.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1599
🏁 Script executed:
#!/bin/bash
# Check the actual call sequence in profile.js to understand when reportError vs reportInfo is called
sed -n '95,115p' src/modules/profile.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1170
reportInfo writes to wzrkError.c and wzrkError.d, overwriting error codes set by reportError.
Both methods modify the same wzrkError object properties. Since cleanNullEmptyValues calls reportInfo up to 5 times during a single validation pass (for null/empty value removals), and validation happens after error-level checks (e.g., isChargedEventStructureValid calls reportError(511) before isObjStructureValid calls cleanNullEmptyValues), the final wzrkError values sent in requests will reflect the last info-level code, silently losing the critical error signal.
Consider either: (a) using a separate property (e.g. wzrkInfo) for informational codes, or (b) not writing to wzrkError in reportInfo at all if the intent is console logging only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/logger.js` around lines 75 - 79, reportInfo currently mutates the
shared wzrkError (this.wzrkError.c/d) which overwrites earlier error codes set
by reportError during validation flows like isChargedEventStructureValid ->
isObjStructureValid -> cleanNullEmptyValues; change reportInfo to stop writing
into this.wzrkError and instead write into a separate informational object
(e.g., this.wzrkInfo.c and this.wzrkInfo.d) or only log to console if info
should not be included in payloads; update the reportInfo implementation to set
this.wzrkInfo rather than this.wzrkError and keep the existing info(...) call,
and ensure any request-building code continues to use this.wzrkError for error
signaling so error-level codes are not lost.
| const gFromCache = $ct.LRU_CACHE.get(kId) | ||
| $ct.LRU_CACHE.set(kId, gFromCache) | ||
| StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) | ||
| this.#device.gcookie = gFromCache | ||
| // Only override gcookie if we don't have a customId | ||
| if (!customIdFlag) { | ||
| this.#device.gcookie = gFromCache | ||
| } |
There was a problem hiding this comment.
Bug: StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) on line 121 overwrites the custom ID in storage even when customIdFlag is true.
The custom ID flow sets gcookie in both memory and storage at lines 191–192, before addToK is called. When the foundInCache branch executes, line 121 unconditionally writes gFromCache to GCOOKIE_NAME in storage, clobbering the custom ID. Only the in-memory this.#device.gcookie is guarded (lines 123–125). This creates an inconsistency: in-memory gcookie holds the custom ID, but storage holds the cached value. On the next page load, the cached (wrong) value will be read back.
🐛 Proposed fix — guard the storage write as well
const gFromCache = $ct.LRU_CACHE.get(kId)
$ct.LRU_CACHE.set(kId, gFromCache)
- StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache)
- // Only override gcookie if we don't have a customId
- if (!customIdFlag) {
+ if (!customIdFlag) {
+ StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache)
this.#device.gcookie = gFromCache
+ }
- }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/userLogin.js` around lines 119 - 125, The storage write
unconditionally overwrites a custom gcookie: when retrieving gFromCache via
$ct.LRU_CACHE.get(kId) you must not call
StorageManager.saveToLSorCookie(GCOOKIE_NAME, gFromCache) if customIdFlag is
true; change the logic around StorageManager.saveToLSorCookie and the
this.#device.gcookie assignment so both the in-memory and persistent store are
only updated when customIdFlag is false (keep the existing guard on
this.#device.gcookie and add the same guard around
StorageManager.saveToLSorCookie).
| describe('customId handling', () => { | ||
| test('should validate customId field when provided', () => { | ||
| validateCustomCleverTapID.mockReturnValue({ isValid: true, sanitizedId: '_w_custom_oul_id' }) | ||
|
|
||
| // Test the validation function directly | ||
| const result = validateCustomCleverTapID('_w_custom_oul_id') | ||
|
|
||
| expect(validateCustomCleverTapID).toHaveBeenCalledWith('_w_custom_oul_id') | ||
| expect(result.isValid).toBe(true) | ||
| expect(result.sanitizedId).toBe('_w_custom_oul_id') | ||
| }) | ||
|
|
||
| test('should handle invalid customId field', () => { | ||
| validateCustomCleverTapID.mockReturnValue({ isValid: false, sanitizedId: null, error: 'Invalid custom ID format' }) | ||
|
|
||
| // Test the validation function directly | ||
| const result = validateCustomCleverTapID('invalid_id') | ||
|
|
||
| expect(validateCustomCleverTapID).toHaveBeenCalledWith('invalid_id') | ||
| expect(result.isValid).toBe(false) | ||
| expect(result.error).toBe('Invalid custom ID format') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
These tests only exercise the mock, not the actual custom ID integration.
Both tests call validateCustomCleverTapID (which is fully mocked) and assert the mock's pre-configured return value. They don't verify any real behavior — e.g., that onUserLogin.push({ Site: { Identity: 'x', customId: 'foo' } }) actually sets the gcookie, persists to storage, or strips customId from the profile payload before firing the request.
Consider replacing these with integration-style tests that push a profile with customId through the OUL flow and assert observable side effects (gcookie assignment, storage writes, request payload shape).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/clevertap.spec.js` around lines 158 - 180, The tests currently only
exercise the mocked validateCustomCleverTapID; replace them with
integration-style tests that exercise the actual OUL flow: call
onUserLogin.push(...) with a profile that includes customId, let the flow run
(mock network/store as needed), then assert the real observable side effects —
gcookie is assigned, storage write(s) occur with the expected values, and the
outgoing profile/request payload does not include customId (i.e., customId is
stripped) before the network request; use the real validateCustomCleverTapID
implementation rather than mocking it so the full behavior is validated.
4 participants
Changes
Describe the key changes in this PR with the Jira Issue reference
Changes to Public Facing API if any
User can now add the key customId to OUL which would set the clevertap id
Please list the impact on the public facing API if any
How Has This Been Tested?
Describe the testing approach and any relevant configurations (e.g., environment, platform)
Checklist
Link to Deployed SDK
Use these url for testing :
https://static.wizrocket.com/staging/<CURRENT_BRANCH_NAME>/js/clevertap.min.jshttps://static.wizrocket.com/staging/<CURRENT_BRANCH_NAME>/js/sw_webpush.min.jsHow to trigger Automations
Just add a empty commit after all your changes are done in the PR with the command
git commit --allow-empty -m "[run-test] Testing Automation"This will trigger the automation suite
Summary by CodeRabbit
New Features
Improvements
Version