setTimeout isn't cleared when auction ends before its timeout

[dugwood]

As reported here: #647 (comment)

For example, I run 2 calls to requestBids(), with a timeout of 1000ms. Prebid.js is already loaded when I call the pbjs.que.push()
Here is the result, the first column is the time relative to my own script:

first placement:
    2 Starting bids: 4 bidders (just before the requestBids call)
  321 appnexus:   0.114   x 0.87  = 0.099   (CPM: 0.25, Time: 316)
  664 criteo:     0.04    x 1     = 0.04    (CPM: 0.25, Time: 655)
  757 rubicon:    0       x 0.885 = 0       (CPM: 0.25, Time: 748)
  866 rubicon:    2.222   x 0.885 = 1.966   (CPM: 0.25, Time: 858)
  870 Handling bids (=bidsBackHandler)
  870 bid #0: appnexus  CPM: 0      Time: 316
  870 bid #1: criteo    CPM: 0      Time: 655
  870 bid #2: rubicon   CPM: 0      Time: 748
  871 bid #3: rubicon   CPM: 1.966  Time: 858
  885 Winner: rubicon (price bucket: 1.90)

second placement:
   11 Starting bids: 4 bidders (just before the requestBids call)
 1098 Handling bids (=bidsBackHandler)
 1098 No bid
 1119 appnexus:   0.368   x 0.87  = 0.32    (CPM: 0.3, Time: 231)
 1288 criteo:     0.17    x 1     = 0.17    (CPM: 0.3, Time: 395)

As you can see: the pbjs.que.push function is run at 2 and 11ms. The first placement works fine, all bidders answer within 1 second.

But the second placement shows that bidsBackHandler is launch at the timeout of the first placement (around 1000ms, here it's 1098ms).

So the second placement is always blocked by the first one. And bids were rendered within 1s: AppNexus answers in 231ms, so it wasn't started at 11ms, but around 1098-231=867ms, just around the first placement run its bidsBackHandler.

[dugwood]

I'm currently working on a fix, to send a clearTimeout at the right moment.

[dugwood]

Also found a bug for the second call of requestBids(): the timeout isn't correctly set, because the argument name isn't cbTimeout but timeout. Hence the timeout is always set to pbjs.bidderTimeout for the second auction.
Currently testing a fix for both issues.

[dugwood]

See: #643 (comment)

I don't understand the need for past auctions, as we remove all «completed», why do we keep the failed ones? This create all sort of bugs...

[mkendall07]

We need to keep them around in case the bid we send to DFP or adserver actually wins the final auction, so in that case renderAd can lookup and find the bid to display.

Also we can't remove bids that have been set and not won because it's possible they could be sent again in the future.

[dugwood]

@mkendall07 I agree on the DFP part, and I'll let the bids available. For the bids not set... what do you mean exactly? If there's no bid (or the bid returns after the timeout), there's no need for those bids anymore, don't you think? I would have removed them in the first place (based on auctionRunning === false and/or adUnitCode isn't being the one currently in auction, so addBidResponse will ignore those bids).

[dugwood]

Should be fixed when PR #735 will be merged.