Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

investigate auction end event not emitting without specifying timeout #3813

Closed
mkendall07 opened this issue May 9, 2019 · 11 comments · Fixed by #3841
Closed

investigate auction end event not emitting without specifying timeout #3813

mkendall07 opened this issue May 9, 2019 · 11 comments · Fixed by #3841
Assignees
Labels

Comments

@mkendall07
Copy link
Member

Type of issue

If you requestBids without specifying a timeout and the default 3 second timeout is used, no auction end event is emitted.

@khatibda
Copy link
Contributor

khatibda commented May 9, 2019

hi - publisher here (Granite Media). to follow up, we use:

pbjs.requestBids({
  adUnitCodes,
});

we assumed it would default to a timeout of 3000ms. most things behave normally on our page as a result of this. however we just discovered that auctionEnd never fires. this is now a new problem because UserID usersync callback seems to be based on this event firing.

if we explicitly call a timeout, then everything fires and works as expected.

pbjs.requestBids({
  adUnitCodes,
  timeout: 3000,
});

lemme know if you need additional info

@jsnellbaker
Copy link
Collaborator

Hi @khatibda,

Do you have a URL available with the setup (and issue) you mentioned above so I can review further? Thanks.

@jsnellbaker jsnellbaker self-assigned this May 13, 2019
@khatibda
Copy link
Contributor

@jsnellbaker. i don't have a live url at present because we resolved why including the timeout property explicitly. are you able to reproduce locally? if not, i can look into pushing a test page with the bug still present.

@jsnellbaker
Copy link
Collaborator

I was trying some ways to artificially induce the scenario, but I was seeing the auctionEnd event emit successfully during those attempts. If you can put something together that reliably reproduces the behavior, I would greatly appreciate it.

@khatibda
Copy link
Contributor

@jsnellbaker here's your test page:
https://www.workandmoney.com/s/best-emerging-college-majors-394f2c2138404506?dbg_tests=pbjsTimeoutOff-enabled&pbjs_debug=true

notice how the auctionEnd doesn't get logged.

if you toggle into the control bucket (with the timeout property explicitly included in requestBids), you should see auctionEnd get logged.

https://www.workandmoney.com/s/best-emerging-college-majors-394f2c2138404506?dbg_tests=pbjsTimeoutOff-control&pbjs_debug=true

@jsnellbaker
Copy link
Collaborator

@khatibda Thanks for providing the test pages. I believe I found the cause for the auctionEnd event to not fire.

Basically, the event is hidden by a flag that's only active when you pass a function in the bidsBackHandler in the pbjs.requestBids function.

In the case of your first page, it's only passing the adUnitCodes property on the requestBids call. Whereas your second page, is passing adUnitCodes, timeout, and bidsBackHandler.

I tried to look through the code in your build file (story.bbfaf41df20c6a33695c.js) while attempting to debug the prebid code on your test page. Is there some extra logic you put together to not require the bidsBackHandler and still allow the prebid data pass to your ad server? Or is the missing property on the test page not expected?

@khatibda
Copy link
Contributor

khatibda commented May 16, 2019

@jsnellbaker you are absolutely right, and we should have micro-tested this double difference. in my local environment, if we include bidsBackHandler and not the timeout property, it works (auctionEnd fires). if we include only the timeout and not the bidsBackHandler, it doesn't work. so this confirms your theory.

that said...does this mean any pub who doesn't include bidsBackHandler isn't firing auctionEnd? isn't that most pubs? seems like a more specific problem, but also a bigger one!

@jsnellbaker
Copy link
Collaborator

I'm not sure that it's most pubs. That said, I'll take a look at the logic and see if it can be handled better.

@stale
Copy link

stale bot commented May 31, 2019

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.

@stale stale bot added the stale label May 31, 2019
@khatibda
Copy link
Contributor

test page is coming down, this feels like it was reviewed/addressed.

@stale stale bot removed the stale label May 31, 2019
@stale
Copy link

stale bot commented Jun 14, 2019

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.

@stale stale bot added the stale label Jun 14, 2019
jsnellbaker added a commit that referenced this issue Jun 18, 2019
…ompletes (#3841)

* move auctionEnd events so it always executes

* remove some commented code
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this issue Aug 1, 2019
…tion completes (prebid#3841)

* move auctionEnd events so it always executes

* remove some commented code
sa1omon pushed a commit to gamoshi/Prebid.js that referenced this issue Nov 28, 2019
…tion completes (prebid#3841)

* move auctionEnd events so it always executes

* remove some commented code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants