Conversation
jaiminpanchal27
left a comment
There was a problem hiding this comment.
@francoroy Modules are introduced in Prebid with this #1177
So you need to update your Adapter by doing some minor changes. you can check here https://github.com/prebid/Prebid.js/pull/1177/files how you can migrate your code.
Also i left some comments on code.
src/adapters/mobfox.js
Outdated
| ajax.ajax(`${BID_REQUEST_BASE_URL}?${queryString}`, { | ||
| success(resp, xhr) { | ||
| if (xhr.getResponseHeader("Content-Type") == "application/json") | ||
| resp = JSON.parse(resp); |
There was a problem hiding this comment.
wrap all JSON.parse in try catch block
# Conflicts: # adapters.json
|
Hi, any progress? |
|
@francoroy Changes looks good. But not able to validate bids. |
|
@jaiminpanchal27 Thanks. I can share my hello world page: |
|
@francoroy Got it. Thanks. |
|
I did find a bit of an issue here... If your server sends back a "no bid available" error, the adapter is still trying to add the bid to the bidmanager. This was likely working when you wrote it, but changes in #1277 (in particular, removing this check) cause an undefined reference now. You can reproduce the issue with this: Since my changes were unintended, I'm replacing the old behavior in #1372... but I don't think this is ideal. If an adapter doesn't have a bid to add, then it shouldn't be calling the |
|
Hi @dbemiller So I don't understand, should I or should I not call addBidResponse when no bid is available? |
|
@francoroy There's a |
|
@dbemiller this is what I already do in my onBidResponseError function: let bidResponse = bidfactory.createBid(CONSTANTS.STATUS.NO_BID, bid); Is it ok then? |
|
@francoroy You're calling addBidResponse twice, and the second call uses an undefined bid. See the attached screenshot & test page. I set a breakpoint right before the page breaks. |
|
@dbemiller Thanks |
|
@francoroy I'm still getting the exact same error. I've attached a screenshot from the same spot, but looking at your code. Your server is responding with a 200, but the response is It reproduces in your unit tests, too: |
|
@dbemiller Thanks, I've fixed it |
dbemiller
left a comment
There was a problem hiding this comment.
Looks good now.
I would recommend expanding your unit tests to catch this case, to prevent future regressions... but I won't require it for merge.
…built * 'master' of https://github.com/prebid/Prebid.js: (95 commits) Specify --browsers when using gulp test --watch (prebid#1420) Added aliases for aol adapter. (prebid#1371) Added MobFox Adapter (prebid#1312) Fixed style error. (prebid#1419) Add native support for Criteo adapter (prebid#999) Update admediaBidAdapter.js (prebid#1395) Increment pre version (prebid#1413) Prebid 0.26.1 Release (prebid#1412) fix prebid#1410 - issue with ie and xhr.timeout (prebid#1411) Lint modules directory (prebid#1404) Set outstream mediaType based on renderer in response (prebid#1391) Fixing the BidAdjustmentEvent fire time (prebid#1399) Fix banner showing up in prebid-core.js (prebid#1388) Mention NodeJS 4.0 dependency in the README (prebid#1386) Increment pre version (prebid#1385) Prebid 0.26.0 Release (prebid#1384) PulsePoint Lite adapter - adding createNew method for aliasing. (prebid#1383) Modernizing build dependencies (prebid#1355) StickyAdsTV bidder adapter update (prebid#1311) Added CPM value as parameter for Vertoz Adapter (prebid#1306) ...


Type of change
Description of change
Be sure to test the integration with your adserver using the Hello World sample page.
Other information