Skip to content

Commit fd62adb

Browse files
jamesnroktclaude
andauthored
feat: Remove blocking identity call in selectPlacements (#1241)
* feat: Remove blocking identity call in selectPlacements * fix: Address PR review comments in roktManager - Replace double-cast `as unknown as number` with `Number()` for httpCode - Add clarifying comment on optimistic identity merge lifetime Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: Remove blocking identity call in selectPlacements --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b2ee111 commit fd62adb

2 files changed

Lines changed: 89 additions & 42 deletions

File tree

src/roktManager.ts

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -305,27 +305,39 @@ export default class RoktManager {
305305
}
306306

307307
if (!isEmpty(newIdentities)) {
308-
// Call identify with the new user identities
308+
// Fire-and-forget identify — best-effort, does not block selectPlacements
309309
try {
310-
await new Promise<void>((resolve, reject) => {
311-
this.identityService.identify({
310+
this.identityService.identify(
311+
{
312312
userIdentities: {
313313
...currentUserIdentities,
314-
...newIdentities
314+
...newIdentities,
315+
},
316+
},
317+
(result) => {
318+
const httpCode = Number(result?.httpCode);
319+
if (httpCode && (httpCode >= 400 || httpCode < 0)) {
320+
this.logger.error(
321+
'Background identify failed with HTTP ' + httpCode
322+
);
315323
}
316-
}, () => {
317-
resolve();
318-
});
319-
});
324+
// Drain any selectPlacements calls that were deferred while
325+
// identify was in-flight. By the time this callback fires,
326+
// identityCallInFlight has already been reset to false.
327+
this.processMessageQueue();
328+
}
329+
);
320330
} catch (error) {
321331
const errorMessage = error instanceof Error ? error.message : JSON.stringify(error);
322-
this.logger.error('Failed to identify user with updated identities: ' + errorMessage);
332+
this.logger.error('Background identify threw an error: ' + errorMessage);
323333
}
324334
}
325-
326-
// Refresh current user identities to ensure we have the latest values before building enrichedAttributes
327-
this.currentUser = this.identityService.getCurrentUser();
328-
const finalUserIdentities = this.currentUser?.getUserIdentities()?.userIdentities || {};
335+
336+
// Optimistic merge: apply the pending identity changes locally so
337+
// enrichedAttributes are correct without waiting for the identify round-trip.
338+
// The next call to selectPlacements re-fetches currentUser (see line above),
339+
// so stale identity state does not persist beyond this invocation.
340+
const finalUserIdentities = { ...currentUserIdentities, ...newIdentities };
329341

330342
this.setUserAttributes(mappedAttributes);
331343

test/jest/roktManager.spec.ts

Lines changed: 64 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,7 +2156,7 @@ describe('RoktManager', () => {
21562156
);
21572157
});
21582158

2159-
it('should log error when identify fails with a 500 but continue execution', async () => {
2159+
it('should log error when identify callback reports an error but continue execution', async () => {
21602160
const kit: Partial<IRoktKit> = {
21612161
launcher: {
21622162
selectPlacements: jest.fn(),
@@ -2171,7 +2171,57 @@ describe('RoktManager', () => {
21712171

21722172
roktManager.kit = kit as IRoktKit;
21732173

2174-
const mockError = { error: 'Identity service error', code: 500 };
2174+
const mockIdentity = {
2175+
getCurrentUser: jest.fn().mockReturnValue({
2176+
getUserIdentities: () => ({
2177+
userIdentities: {
2178+
email: 'old@example.com'
2179+
}
2180+
}),
2181+
setUserAttributes: jest.fn()
2182+
}),
2183+
identify: jest.fn().mockImplementation((data, callback) => {
2184+
// Simulate identify completing with an HTTP 500 error via the callback
2185+
callback({ httpCode: 500 });
2186+
})
2187+
} as unknown as SDKIdentityApi;
2188+
2189+
roktManager['identityService'] = mockIdentity;
2190+
2191+
const options: IRoktSelectPlacementsOptions = {
2192+
attributes: {
2193+
email: 'new@example.com'
2194+
}
2195+
};
2196+
2197+
// Should not reject since identify is fire-and-forget
2198+
await expect(roktManager.selectPlacements(options)).resolves.toBeDefined();
2199+
2200+
// Verify error was logged from the background callback
2201+
expect(mockMPInstance.Logger.error).toHaveBeenCalledWith(
2202+
'Background identify failed with HTTP 500'
2203+
);
2204+
2205+
// Verify selectPlacements was still called
2206+
expect(kit.selectPlacements).toHaveBeenCalled();
2207+
});
2208+
2209+
it('should log error and continue when identify throws synchronously', async () => {
2210+
const kit: Partial<IRoktKit> = {
2211+
launcher: {
2212+
selectPlacements: jest.fn(),
2213+
hashAttributes: jest.fn(),
2214+
use: jest.fn(),
2215+
},
2216+
selectPlacements: jest.fn().mockResolvedValue({}),
2217+
hashAttributes: jest.fn(),
2218+
setExtensionData: jest.fn(),
2219+
use: jest.fn(),
2220+
};
2221+
2222+
roktManager.kit = kit as IRoktKit;
2223+
2224+
const mockError = new Error('Identity service unavailable');
21752225
const mockIdentity = {
21762226
getCurrentUser: jest.fn().mockReturnValue({
21772227
getUserIdentities: () => ({
@@ -2194,12 +2244,12 @@ describe('RoktManager', () => {
21942244
}
21952245
};
21962246

2197-
// Should not reject since we're catching and logging the error
2247+
// Should not reject — the try/catch absorbs the synchronous throw
21982248
await expect(roktManager.selectPlacements(options)).resolves.toBeDefined();
2199-
2200-
// Verify error was logged
2249+
2250+
// Verify the synchronous throw was logged
22012251
expect(mockMPInstance.Logger.error).toHaveBeenCalledWith(
2202-
'Failed to identify user with updated identities: ' + JSON.stringify(mockError)
2252+
'Background identify threw an error: ' + mockError.message
22032253
);
22042254

22052255
// Verify selectPlacements was still called
@@ -2358,7 +2408,7 @@ describe('RoktManager', () => {
23582408
expect(kit.selectPlacements).toHaveBeenCalled();
23592409
});
23602410

2361-
it('should update filters.filteredUser with newly identified user when anonymous user is identified through selectPlacements', async () => {
2411+
it('should set filters.filteredUser with the current user at call time when identify is fired in background', async () => {
23622412
const anonymousUser = {
23632413
getMPID: () => 'anonymous-mpid',
23642414
getUserIdentities: () => ({
@@ -2367,16 +2417,6 @@ describe('RoktManager', () => {
23672417
setUserAttributes: jest.fn()
23682418
} as unknown as IMParticleUser;
23692419

2370-
const identifiedUser = {
2371-
getMPID: () => testMPID,
2372-
getUserIdentities: () => ({
2373-
userIdentities: {
2374-
email: 'user@example.com'
2375-
}
2376-
}),
2377-
setUserAttributes: jest.fn()
2378-
} as unknown as IMParticleUser;
2379-
23802420
const kit: Partial<IRoktKit> = {
23812421
launcher: {
23822422
selectPlacements: jest.fn(),
@@ -2395,13 +2435,9 @@ describe('RoktManager', () => {
23952435
roktManager.filters.filteredUser = anonymousUser;
23962436

23972437
const mockIdentity = {
2398-
getCurrentUser: jest.fn()
2399-
// First call: anonymous user (before identify)
2400-
.mockReturnValueOnce(anonymousUser)
2401-
// Second call: identified user (after identify completes)
2402-
.mockReturnValue(identifiedUser),
2438+
getCurrentUser: jest.fn().mockReturnValue(anonymousUser),
24032439
identify: jest.fn().mockImplementation((data, callback) => {
2404-
// Simulate identify completing and updating the user
2440+
// Simulate identify completing in background
24052441
callback();
24062442
})
24072443
} as unknown as SDKIdentityApi;
@@ -2416,19 +2452,18 @@ describe('RoktManager', () => {
24162452

24172453
await roktManager.selectPlacements(options);
24182454

2419-
// Verify identify was called
2455+
// Verify identify was fired
24202456
expect(mockIdentity.identify).toHaveBeenCalledWith({
24212457
userIdentities: {
24222458
email: 'user@example.com'
24232459
}
24242460
}, expect.any(Function));
24252461

2426-
// Verify filters.filteredUser was updated with the newly identified user
2427-
expect(roktManager.filters.filteredUser).toBe(identifiedUser);
2428-
expect(roktManager.filters.filteredUser).not.toBe(anonymousUser);
2429-
expect(roktManager.filters.filteredUser?.getMPID()).toBe(testMPID);
2462+
// filters.filteredUser reflects the user at call time (before identify completes)
2463+
expect(roktManager.filters.filteredUser).toBe(anonymousUser);
2464+
expect(roktManager.filters.filteredUser?.getMPID()).toBe('anonymous-mpid');
24302465

2431-
// Verify kit was called
2466+
// Verify kit was still called
24322467
expect(kit.selectPlacements).toHaveBeenCalled();
24332468
});
24342469

0 commit comments

Comments
 (0)