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

UserID module: graceful handling of exceptions from ID submodules #8401

Merged
merged 2 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
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
15 changes: 12 additions & 3 deletions modules/id5IdSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@
* @requires module:modules/userId
*/

import { deepAccess, logInfo, deepSetValue, logError, isEmpty, isEmptyStr, logWarn } from '../src/utils.js';
import {
deepAccess,
logInfo,
deepSetValue,
logError,
isEmpty,
isEmptyStr,
logWarn,
safeJSONParse
} from '../src/utils.js';
import { ajax } from '../src/ajax.js';
import { submodule } from '../src/hook.js';
import { getRefererInfo } from '../src/refererDetection.js';
Expand All @@ -24,7 +33,7 @@ const LOG_PREFIX = 'User ID - ID5 submodule: ';
// cookie in the array is the most preferred to use
const LEGACY_COOKIE_NAMES = [ 'pbjs-id5id', 'id5id.1st', 'id5id' ];

const storage = getStorageManager({gvlid: GVLID, moduleName: MODULE_NAME});
export const storage = getStorageManager({gvlid: GVLID, moduleName: MODULE_NAME});

/** @type {Submodule} */
export const id5IdSubmodule = {
Expand Down Expand Up @@ -253,7 +262,7 @@ function getLegacyCookieSignature() {
let legacyStoredValue;
LEGACY_COOKIE_NAMES.forEach(function(cookie) {
if (storage.getCookie(cookie)) {
legacyStoredValue = JSON.parse(storage.getCookie(cookie)) || legacyStoredValue;
legacyStoredValue = safeJSONParse(storage.getCookie(cookie)) || legacyStoredValue;
}
});
return (legacyStoredValue && legacyStoredValue.signature) || '';
Expand Down
19 changes: 14 additions & 5 deletions modules/userId/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ function processSubmoduleCallbacks(submodules, cb) {
}, submodules.length);
}
submodules.forEach(function (submodule) {
submodule.callback(function callbackCompleted(idObj) {
function callbackCompleted(idObj) {
// if valid, id data should be saved to cookie/html storage
if (idObj) {
if (submodule.config.storage) {
Expand All @@ -435,8 +435,13 @@ function processSubmoduleCallbacks(submodules, cb) {
logInfo(`${MODULE_NAME}: ${submodule.submodule.name} - request id responded with an empty value`);
}
done();
});

}
try {
submodule.callback(callbackCompleted);
} catch (e) {
logError(`Error in userID module '${submodule.submodule.name}':`, e);
done();
}
// clear callback, this prop is used to test if all submodule callbacks are complete below
submodule.callback = undefined;
});
Expand Down Expand Up @@ -881,8 +886,12 @@ function initSubmodules(dest, submodules, consentData, forceRefresh = false) {
setStoredConsentData(consentData);

const initialized = userIdModules.reduce((carry, submodule) => {
populateSubmoduleId(submodule, consentData, storedConsentData, forceRefresh);
carry.push(submodule);
try {
populateSubmoduleId(submodule, consentData, storedConsentData, forceRefresh);
carry.push(submodule);
} catch (e) {
logError(`Error in userID module '${submodule.submodule.name}':`, e);
}
return carry;
}, []);
if (initialized.length) {
Expand Down
11 changes: 11 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1364,3 +1364,14 @@ export function cyrb53Hash(str, seed = 0) {
export function getWindowFromDocument(doc) {
return (doc) ? doc.defaultView : null;
}

/**
* returns the result of `JSON.parse(data)`, or undefined if that throws an error.
* @param data
* @returns {any}
*/
export function safeJSONParse(data) {
try {
return JSON.parse(data);
} catch (e) {}
}
17 changes: 16 additions & 1 deletion test/spec/modules/id5IdSystem_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
ID5_PRIVACY_STORAGE_NAME,
ID5_STORAGE_NAME,
id5IdSubmodule,
nbCacheName,
nbCacheName, storage,
storeInLocalStorage,
storeNbInCache,
} from 'modules/id5IdSystem.js';
Expand Down Expand Up @@ -310,6 +310,21 @@ describe('ID5 ID System', function() {
request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
expect(getFromLocalStorage(ID5_PRIVACY_STORAGE_NAME)).to.be.null;
});

describe('when legacy cookies are set', () => {
let sandbox;
beforeEach(() => {
sandbox = sinon.sandbox.create();
sandbox.stub(storage, 'getCookie');
});
afterEach(() => {
sandbox.restore();
});
it('should not throw if malformed JSON is forced into cookies', () => {
storage.getCookie.callsFake(() => ' Not JSON ');
id5IdSubmodule.getId(getId5FetchConfig());
});
})
});

describe('Request Bids Hook', function() {
Expand Down
51 changes: 51 additions & 0 deletions test/spec/modules/userId_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,57 @@ describe('User ID', function () {
});
});

describe('when ID systems throw errors', () => {
function mockIdSystem(name) {
return {
name,
decode: function(value) {
return {
[name]: value
};
},
getId: sinon.stub().callsFake(() => ({id: name}))
};
}
let id1, id2;
beforeEach(() => {
id1 = mockIdSystem('mock1');
id2 = mockIdSystem('mock2');
init(config);
setSubmoduleRegistry([id1, id2]);
config.setConfig({
userSync: {
auctionDelay: 10,
userIds: [{
name: 'mock1',
storage: {name: 'mock1', type: 'cookie'}
}, {
name: 'mock2',
storage: {name: 'mock2', type: 'cookie'}
}]
}
})
});
afterEach(() => {
config.resetConfig();
})
Object.entries({
'in init': () => id1.getId.callsFake(() => { throw new Error() }),
'in callback': () => {
const mockCallback = sinon.stub().callsFake(() => { throw new Error() });
id1.getId.callsFake(() => ({callback: mockCallback}))
}
}).forEach(([t, setup]) => {
describe(`${t}`, () => {
beforeEach(setup);
it('should still retrieve IDs that do not throw', () => {
return getGlobal().getUserIdsAsync().then((uid) => {
expect(uid.mock2).to.not.be.undefined;
})
});
})
})
});
it('pbjs.refreshUserIds updates submodules', function(done) {
let sandbox = sinon.createSandbox();
let mockIdCallback = sandbox.stub().returns({id: {'MOCKID': '1111'}});
Expand Down