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

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

Merged
merged 5 commits into from
Jul 19, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
22 changes: 11 additions & 11 deletions src/consentHandler.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
import {isStr, timestamp} from './utils.js';
import {defer, GreedyPromise} from './utils/promise.js';

export class ConsentHandler {
#enabled;
#data;
#promise;
#resolve;
#defer;
#ready;
generatedTime;

constructor() {
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;
@@ -56,12 +56,12 @@ 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;
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.