User syncs only fired with the first pbjs.requestBids()

[jbartek25]

Type of issue

Bug

Description

User syncs are currently only fired once on the first call of pbjs.requestBids(). This is normally fine if pbjs.requestBids() is executed just once or if all ad units on the same page use the same set of bidders. However, we often have on the same page mix of banner and instream video ad units with different set of video-only bidders (there's obviously some overlap). Banners are requested during the page load, video ads are fetched (pre-fetched) via a separate pbjs.requestBids() call(s). What we have noticed is user syncs never get fired for video bidders, only for banner bidders unless obviously we explicitly trigger user syncing via pbjs.triggerUserSyncs().
We believe this is a bug and a correct behaviour should be something like this:

• Fire syncs after the first pbjs.requestBids(), remember all bidders for which the syncs were fired.
• On next pbjs.requestBids() fire syncs for any new bidder/bidder alias

Test page

Here's a bit artificial example with 2 banners. The upper banner has a single bidder Improve Digital, the bottom banner Appnexus. If you load the upper banner first, syncs from Improve Digital will be fired. Then if you click the button to load the bottom banner, you should see Appnexus iframe syncs fired but they never are.

Platform details

Prebid v2.10

[mkendall07]

@idettman can you comment on this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jbartek25]

@idettman would you please comment?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jbartek25]

ping

[snapwich]

Sounds like a bug to me.

All syncs coming from the bidderAdapter's getUserSyncs are registered on this userSync instance created here: https://github.com/prebid/Prebid.js/blob/master/src/userSync.js#L281

However, each userSync instance keeps a flag on whether it's fired all its syncs (as a whole) and then won't fire again if that flag is set.

I'm not sure what responsibilities the userSync instance was originally intended to have, but if it were to track all userSyncs then it seems like it seems like it shouldn't have this flag and should be allowed to fire multiple times while removing previously fired items out of the queue. If it's only suppose to track userSyncs per auction, then a separate userSync instance should probably be created for each auction. Thoughts @idettman or @jaiminpanchal27 ?

[snapwich]

It looks like the primary responsibilities of the userSync code is to enable/disabled, filter, and limit syncs across the board and per bidder. If that's the case it probably makes sense in creating separate userSync instances per auction since I don't think we want the syncs fired from one auction limiting syncs from another auction?

[GLStephen]

Is there a technical reason to have the same bidder sync on multiple auctions? Do bidders currently each sync multiple times in the same auction for some reason?

I think that many people may be expecting the userSync code (at least in part) to limit the total syncs and the associated requests, bandwidth, page load, etc. Given that, it would make sense to still treat it globally and ensure that each bidder gets a sync until the global limit is met. I think it would be best to limit the total syncs and to treat each auction as an opportunity for a new bidder to sync.

The alternative, in a lazy load situation for instances, with many auctions per page is a multiplication of user syncs that seem largely unnecessary.

[snapwich]

Yeah I think I agree with you here @GLStephen, I think this logic needs to be updated.

There is #3320 that now has the syncs firing for each auction unless enableOverride is enabled, but as you mention that will cause syncs to fire multiple times for the same bidder. The bug here is that a bidder that wasn't included in the first auction might never get synced in their auction that happens later on the same page, so while #3220 might fix that, now we'll have the issue of multiple syncs firing; which is not ideal...

I think this needs to be reworked.