Skip to content

Commit

Permalink
Core & multiple modules: refactor usage of Promise to avoid deferrals…
Browse files Browse the repository at this point in the history
… when possible (prebid#8626)

* Core & multiple modules: refactor usage of Promise to avoid deferrals when possible

* Unhandled rejection handling

* Improve tests
  • Loading branch information
dgirardi authored and jorgeluisrocha committed May 18, 2023

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
1 parent d5fb149 commit 5868553
Showing 17 changed files with 476 additions and 196 deletions.
4 changes: 2 additions & 2 deletions modules/currency.js
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ import CONSTANTS from '../src/constants.json';
import { ajax } from '../src/ajax.js';
import { config } from '../src/config.js';
import { getHook } from '../src/hook.js';
import {promiseControls} from '../src/utils/promise.js';
import {defer} from '../src/utils/promise.js';

const DEFAULT_CURRENCY_RATE_URL = 'https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json?date=$$TODAY$$';
const CURRENCY_RATE_PRECISION = 4;
@@ -24,7 +24,7 @@ var defaultRates;
export const ready = (() => {
let ctl;
function reset() {
ctl = promiseControls();
ctl = defer();
}
reset();
return {done: () => ctl.resolve(), reset, promise: () => ctl.promise}
60 changes: 39 additions & 21 deletions modules/userId/index.js
Original file line number Diff line number Diff line change
@@ -152,7 +152,7 @@ import {
isEmpty
} from '../../src/utils.js';
import {getPPID as coreGetPPID} from '../../src/adserver.js';
import {promiseControls} from '../../src/utils/promise.js';
import {defer, GreedyPromise} from '../../src/utils/promise.js';
import {hasPurpose1Consent} from '../../src/utils/gpdr.js';

const MODULE_NAME = 'User ID';
@@ -504,24 +504,20 @@ function addIdDataToAdUnitBids(adUnits, submodules) {
});
}

function delayFor(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

const INIT_CANCELED = {};

function idSystemInitializer({delay = delayFor} = {}) {
const startInit = promiseControls();
const startCallbacks = promiseControls();
function idSystemInitializer({delay = GreedyPromise.timeout} = {}) {
const startInit = defer();
const startCallbacks = defer();
let cancel;
let initialized = false;

function cancelAndTry(promise) {
if (cancel != null) {
cancel.reject(INIT_CANCELED);
}
cancel = promiseControls();
return Promise.race([promise, cancel.promise]);
cancel = defer();
return GreedyPromise.race([promise, cancel.promise]);
}

// grab a reference to global vars so that the promise chains remain isolated;
@@ -540,7 +536,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
}

let done = cancelAndTry(
Promise.all([hooksReady, startInit.promise])
GreedyPromise.all([hooksReady, startInit.promise])
.then(() => gdprDataHandler.promise)
.then(checkRefs((consentData) => {
initSubmodules(initModules, allModules, consentData);
@@ -549,7 +545,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
.then(checkRefs(() => {
const modWithCb = initModules.filter(item => isFn(item.callback));
if (modWithCb.length) {
return new Promise((resolve) => processSubmoduleCallbacks(modWithCb, resolve));
return new GreedyPromise((resolve) => processSubmoduleCallbacks(modWithCb, resolve));
}
}))
);
@@ -573,7 +569,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
});
}
}
if (refresh) {
if (refresh && initialized) {
done = cancelAndTry(
done
.catch(() => null)
@@ -588,7 +584,7 @@ function idSystemInitializer({delay = delayFor} = {}) {
return sm.callback != null;
});
if (cbModules.length) {
return new Promise((resolve) => processSubmoduleCallbacks(cbModules, resolve));
return new GreedyPromise((resolve) => processSubmoduleCallbacks(cbModules, resolve));
}
}))
);
@@ -621,8 +617,8 @@ function getPPID() {
* @param {Object} reqBidsConfigObj required; This is the same param that's used in pbjs.requestBids.
* @param {function} fn required; The next function in the chain, used by hook.js
*/
export function requestBidsHook(fn, reqBidsConfigObj, {delay = delayFor} = {}) {
Promise.race([
export function requestBidsHook(fn, reqBidsConfigObj, {delay = GreedyPromise.timeout} = {}) {
GreedyPromise.race([
getUserIdsAsync(),
delay(auctionDelay)
]).then(() => {
@@ -767,7 +763,16 @@ function refreshUserIds({submoduleNames} = {}, callback) {
*/

function getUserIdsAsync() {
return initIdSystem().then(() => getUserIds(), (e) => e === INIT_CANCELED ? getUserIdsAsync() : Promise.reject(e));
return initIdSystem().then(
() => getUserIds(),
(e) =>
e === INIT_CANCELED
// there's a pending refresh - because GreedyPromise runs this synchronously, we are now in the middle
// of canceling the previous init, before the refresh logic has had a chance to run.
// Use a "normal" Promise to clear the stack and let it complete (or this will just recurse infinitely)
? Promise.resolve().then(getUserIdsAsync)
: GreedyPromise.reject(e)
);
}

/**
@@ -914,6 +919,8 @@ function updateSubmodules() {
return;
}
// do this to avoid reprocessing submodules
// TODO: the logic does not match the comment - addedSubmodules is always a copy of submoduleRegistry
// (if it did it would not be correct - it's not enough to find new modules, as others may have been removed or changed)
const addedSubmodules = submoduleRegistry.filter(i => !find(submodules, j => j.name === i.name));

submodules.splice(0, submodules.length);
@@ -949,6 +956,17 @@ export function attachIdSystem(submodule) {
if (!find(submoduleRegistry, i => i.name === submodule.name)) {
submoduleRegistry.push(submodule);
updateSubmodules();
// TODO: a test case wants this to work even if called after init (the setConfig({userId}))
// so we trigger a refresh. But is that even possible outside of tests?
initIdSystem({refresh: true, submoduleNames: [submodule.name]});
}
}

function normalizePromise(fn) {
// for public methods that return promises, make sure we return a "normal" one - to avoid
// exposing confusing stack traces
return function() {
return Promise.resolve(fn.apply(this, arguments));
}
}

@@ -957,7 +975,7 @@ export function attachIdSystem(submodule) {
* so a callback is added to fire after the consentManagement module.
* @param {{getConfig:function}} config
*/
export function init(config, {delay = delayFor} = {}) {
export function init(config, {delay = GreedyPromise.timeout} = {}) {
ppidSource = undefined;
submodules = [];
configRegistry = [];
@@ -1002,10 +1020,10 @@ export function init(config, {delay = delayFor} = {}) {
// exposing getUserIds function in global-name-space so that userIds stored in Prebid can be used by external codes.
(getGlobal()).getUserIds = getUserIds;
(getGlobal()).getUserIdsAsEids = getUserIdsAsEids;
(getGlobal()).getEncryptedEidsForSource = getEncryptedEidsForSource;
(getGlobal()).getEncryptedEidsForSource = normalizePromise(getEncryptedEidsForSource);
(getGlobal()).registerSignalSources = registerSignalSources;
(getGlobal()).refreshUserIds = refreshUserIds;
(getGlobal()).getUserIdsAsync = getUserIdsAsync;
(getGlobal()).refreshUserIds = normalizePromise(refreshUserIds);
(getGlobal()).getUserIdsAsync = normalizePromise(getUserIdsAsync);
(getGlobal()).getUserIdsAsEidBySource = getUserIdsAsEidBySource;
}

11 changes: 6 additions & 5 deletions src/auction.js
Original file line number Diff line number Diff line change
@@ -76,6 +76,7 @@ import {bidderSettings} from './bidderSettings.js';
import * as events from './events.js'
import adapterManager from './adapterManager.js';
import CONSTANTS from './constants.json';
import {GreedyPromise} from './utils/promise.js';

const { syncUsers } = userSync;

@@ -384,9 +385,9 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM

function waitFor(requestId, result) {
if (ready[requestId] == null) {
ready[requestId] = Promise.resolve();
ready[requestId] = GreedyPromise.resolve();
}
ready[requestId] = ready[requestId].then(() => Promise.resolve(result).catch(() => {}))
ready[requestId] = ready[requestId].then(() => GreedyPromise.resolve(result).catch(() => {}))
}

function guard(bidderRequest, fn) {
@@ -398,9 +399,9 @@ export function auctionCallbacks(auctionDone, auctionInstance, {index = auctionM
const wait = ready[bidderRequest.bidderRequestId];
const orphanWait = ready['']; // also wait for "orphan" responses that are not associated with any request
if ((wait != null || orphanWait != null) && timeRemaining > 0) {
Promise.race([
new Promise((resolve) => setTimeout(resolve, timeRemaining)),
Promise.resolve(orphanWait).then(() => wait)
GreedyPromise.race([
GreedyPromise.timeout(timeRemaining),
GreedyPromise.resolve(orphanWait).then(() => wait)
]).then(fn);
} else {
fn();
29 changes: 16 additions & 13 deletions src/consentHandler.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
import {isStr, timestamp} from './utils.js';
import {defer, GreedyPromise} from './utils/promise.js';

export class ConsentHandler {
enabled;
data;
defer;
ready;
generatedTime;

constructor() {
this.enabled = false;
this.ready = false;
this._promise = null;
this.reset();
}

resolve(data) {
this.ready = true;
this.data = data;
this.defer.resolve(data);
}

/**
* reset this handler (mainly for tests)
*/
reset() {
this.promise = new Promise((resolve) => {
this.resolve = (data) => {
this.ready = true;
this.data = data;
resolve(data);
};
});
this.defer = defer();
this.enabled = false;
this.data = null;
this.ready = false;
@@ -38,17 +45,13 @@ export class ConsentHandler {
*/
get promise() {
if (this.ready) {
return Promise.resolve(this.data);
return GreedyPromise.resolve(this.data);
}

if (!this.enabled) {
this.resolve(null);
}
return this._promise;
}

set promise(prom) {
this._promise = prom;
return this.defer.promise;
}

setConsentData(data, time = timestamp()) {
8 changes: 5 additions & 3 deletions src/debugging.js
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ import {getGlobal} from './prebidGlobal.js';
import {logMessage, prefixLog} from './utils.js';
import {createBid} from './bidfactory.js';
import {loadExternalScript} from './adloader.js';
import {GreedyPromise} from './utils/promise.js';

export const DEBUG_KEY = '__$$PREBID_GLOBAL$$_debugging__';

@@ -12,7 +13,7 @@ function isDebuggingInstalled() {
}

function loadScript(url) {
return new Promise((resolve) => {
return new GreedyPromise((resolve) => {
loadExternalScript(url, 'debugging', resolve);
});
}
@@ -21,7 +22,8 @@ export function debuggingModuleLoader({alreadyInstalled = isDebuggingInstalled,
let loading = null;
return function () {
if (loading == null) {
loading = new Promise((resolve, reject) => {
loading = new GreedyPromise((resolve, reject) => {
// run this in a 0-delay timeout to give installedModules time to be populated
setTimeout(() => {
if (alreadyInstalled()) {
resolve();
@@ -44,7 +46,7 @@ export function debuggingControls({load = debuggingModuleLoader(), hook = getHoo
let promise = null;
let enabled = false;
function waitForDebugging(next, ...args) {
return (promise || Promise.resolve()).then(() => next.apply(this, args))
return (promise || GreedyPromise.resolve()).then(() => next.apply(this, args))
}
function enable() {
if (!enabled) {
4 changes: 2 additions & 2 deletions src/hook.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import funHooks from 'fun-hooks/no-eval/index.js';
import {promiseControls} from './utils/promise.js';
import {defer} from './utils/promise.js';

export let hook = funHooks({
ready: funHooks.SYNC | funHooks.ASYNC | funHooks.QUEUE
});

const readyCtl = promiseControls();
const readyCtl = defer();
hook.ready = (() => {
const ready = hook.ready;
return function () {
5 changes: 3 additions & 2 deletions src/utils.js
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ import { config } from './config.js';
import clone from 'just-clone';
import {find, includes} from './polyfill.js';
import CONSTANTS from './constants.json';
import {GreedyPromise} from './utils/promise.js';
export { default as deepAccess } from 'dlv/index.js';
export { default as deepSetValue } from 'dset';

@@ -487,7 +488,7 @@ export function hasOwn(objectToCheck, propertyToCheckFor) {
* @param {HTMLElement} [doc]
* @param {HTMLElement} [target]
* @param {Boolean} [asLastChildChild]
* @return {HTMLElement}
* @return {HTML Element}
*/
export function insertElement(elm, doc, target, asLastChildChild) {
doc = doc || document;
@@ -517,7 +518,7 @@ export function insertElement(elm, doc, target, asLastChildChild) {
*/
export function waitForElementToLoad(element, timeout) {
let timer = null;
return new Promise((resolve) => {
return new GreedyPromise((resolve) => {
const onLoad = function() {
element.removeEventListener('load', onLoad);
element.removeEventListener('error', onLoad);
Loading
Oops, something went wrong.

0 comments on commit 5868553

Please sign in to comment.