Bug 1644743 - Consolidate Targeting evaluation across Normandy, Experiment Manager and ASRouter r=k88hudson,mythmon,janerik

Differential Revision: https://phabricator.services.mozilla.com/D79967
This commit is contained in:
Andrei Oprea 2020-07-24 12:12:10 +00:00
parent ca9407d143
commit bb5054fefb
15 changed files with 582 additions and 169 deletions

View File

@ -1482,3 +1482,23 @@ Unlike other Activity Stream pings, this is a Firefox Events telemetry event, an
}
}
```
### Experiment attribute errors
This records whether issues were encountered with any of the targeting attributes used in the experiment enrollment or message targeting.
Two different types of events are sent: `attribute_error` and `attribute_timeout` along with the attribute that caused it.
```js
[
"messaging_experiments",
"targeting",
"attribute_error", // event
"foo" // attribute
],
[
"messaging_experiments",
"targeting",
"attribute_timeout", // event
"bar" // attribute
]
```

View File

@ -34,6 +34,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
ExperimentAPI: "resource://messaging-system/experiments/ExperimentAPI.jsm",
SpecialMessageActions:
"resource://messaging-system/lib/SpecialMessageActions.jsm",
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
});
XPCOMUtils.defineLazyServiceGetters(this, {
BrowserHandler: ["@mozilla.org/browser/clh;1", "nsIBrowserHandler"],
@ -581,12 +582,13 @@ class _ASRouter {
// Notify all tabs of messages that have become invalid after pref change
const invalidMessages = [];
const context = this._getMessagesContext();
const targetingContext = new TargetingContext(context);
for (const msg of this.state.messages.filter(this.isUnblockedMessage)) {
if (!msg.targeting) {
continue;
}
const isMatch = await ASRouterTargeting.isMatch(msg.targeting, context);
const isMatch = await targetingContext.evalWithDefault(msg.targeting);
if (!isMatch) {
invalidMessages.push(msg.id);
}
@ -1105,7 +1107,7 @@ class _ASRouter {
});
}
_handleTargetingError(type, error, message) {
_handleTargetingError(error, message) {
Cu.reportError(error);
if (this.dispatchToAS) {
this.dispatchToAS(
@ -1113,7 +1115,6 @@ class _ASRouter {
message_id: message.id,
action: "asrouter_undesired_event",
event: "TARGETING_EXPRESSION_ERROR",
event_context: type,
})
);
}
@ -1147,10 +1148,11 @@ class _ASRouter {
async evaluateExpression(target, { expression, context }) {
const channel = target || this.messageChannel;
const targetingContext = new TargetingContext(context);
let evaluationStatus;
try {
evaluationStatus = {
result: await ASRouterTargeting.isMatch(expression, context),
result: await targetingContext.evalWithDefault(expression),
success: true,
};
} catch (e) {

View File

@ -21,8 +21,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.jsm",
AppConstants: "resource://gre/modules/AppConstants.jsm",
AttributionCode: "resource:///modules/AttributionCode.jsm",
FilterExpressions:
"resource://gre/modules/components-utils/FilterExpressions.jsm",
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
fxAccounts: "resource://gre/modules/FxAccounts.jsm",
Region: "resource://gre/modules/Region.jsm",
});
@ -101,7 +100,6 @@ XPCOMUtils.defineLazyServiceGetter(
);
const FXA_USERNAME_PREF = "services.sync.username";
const MOZ_JEXL_FILEPATH = "mozjexl";
const { activityStreamProvider: asProvider } = NewTabUtils;
@ -566,37 +564,6 @@ const TargetingGetters = {
this.ASRouterTargeting = {
Environment: TargetingGetters,
ERROR_TYPES: {
MALFORMED_EXPRESSION: "MALFORMED_EXPRESSION",
ATTRIBUTE_ERROR: "JEXL_ATTRIBUTE_GETTER_ERROR",
OTHER_ERROR: "OTHER_ERROR",
},
// Combines the getter properties of two objects without evaluating them
combineContexts(contextA = {}, contextB = {}, onError) {
return {
get: (obj, prop) => {
try {
return contextA[prop] || contextB[prop];
} catch (error) {
onError(this.ERROR_TYPES.ATTRIBUTE_ERROR, error, prop);
}
return null;
},
};
},
isMatch(filterExpression, customContext, onError) {
return FilterExpressions.eval(
filterExpression,
new Proxy(
{},
this.combineContexts(customContext, this.Environment, onError)
)
);
},
isTriggerMatch(trigger = {}, candidateMessageTrigger = {}) {
if (trigger.id !== candidateMessageTrigger.id) {
return false;
@ -650,12 +617,12 @@ this.ASRouterTargeting = {
* checkMessageTargeting - Checks is a message's targeting parameters are satisfied
*
* @param {*} message An AS router message
* @param {obj} context A FilterExpression context
* @param {obj} targetingContext a TargetingContext instance complete with eval environment
* @param {func} onError A function to handle errors (takes two params; error, message)
* @param {boolean} shouldCache Should the JEXL evaluations be cached and reused.
* @returns
*/
async checkMessageTargeting(message, context, onError, shouldCache) {
async checkMessageTargeting(message, targetingContext, onError, shouldCache) {
// If no targeting is specified,
if (!message.targeting) {
return true;
@ -668,7 +635,7 @@ this.ASRouterTargeting = {
return result.value;
}
}
result = await this.isMatch(message.targeting, context, onError);
result = await targetingContext.evalWithDefault(message.targeting);
if (shouldCache) {
jexlEvaluationCache.set(message.targeting, {
timestamp: Date.now(),
@ -676,19 +643,22 @@ this.ASRouterTargeting = {
});
}
} catch (error) {
Cu.reportError(error);
if (onError) {
const type = error.fileName.includes(MOZ_JEXL_FILEPATH)
? this.ERROR_TYPES.MALFORMED_EXPRESSION
: this.ERROR_TYPES.OTHER_ERROR;
onError(type, error, message);
onError(error, message);
}
Cu.reportError(error);
result = false;
}
return result;
},
_isMessageMatch(message, trigger, context, onError, shouldCache = false) {
_isMessageMatch(
message,
trigger,
targetingContext,
onError,
shouldCache = false
) {
return (
message &&
(trigger
@ -696,7 +666,12 @@ this.ASRouterTargeting = {
: !message.trigger) &&
// If a trigger expression was passed to this function, the message should match it.
// Otherwise, we should choose a message with no trigger property (i.e. a message that can show up at any time)
this.checkMessageTargeting(message, context, onError, shouldCache)
this.checkMessageTargeting(
message,
targetingContext,
onError,
shouldCache
)
);
},
@ -715,25 +690,28 @@ this.ASRouterTargeting = {
*/
async findMatchingMessage({
messages,
trigger,
context,
trigger = {},
context = {},
onError,
ordered = false,
shouldCache = false,
returnAll = false,
}) {
const sortedMessages = getSortedMessages(messages, { ordered });
const combinedContext = new Proxy(
{},
this.combineContexts(trigger && trigger.context, context, onError)
);
const matching = returnAll ? [] : null;
const targetingContext = new TargetingContext(
TargetingContext.combineContexts(
context,
this.Environment,
trigger.context || {}
)
);
const isMatch = candidate =>
this._isMessageMatch(
candidate,
trigger,
combinedContext,
targetingContext,
onError,
shouldCache
);

View File

@ -48,33 +48,6 @@ ChromeUtils.defineModuleGetter(
"resource://gre/modules/Region.jsm"
);
// ASRouterTargeting.isMatch
add_task(async function should_do_correct_targeting() {
is(
await ASRouterTargeting.isMatch("FOO", { FOO: true }),
true,
"should return true for a matching value"
);
is(
await ASRouterTargeting.isMatch("!FOO", { FOO: true }),
false,
"should return false for a non-matching value"
);
});
add_task(async function should_handle_async_getters() {
const context = {
get FOO() {
return Promise.resolve(true);
},
};
is(
await ASRouterTargeting.isMatch("FOO", context),
true,
"should return true for a matching async value"
);
});
// ASRouterTargeting.findMatchingMessage
add_task(async function find_matching_message() {
const messages = [
@ -107,38 +80,10 @@ add_task(async function return_nothing_for_no_matching_message() {
);
});
add_task(async function check_syntax_error_handling() {
let result;
function onError(...args) {
result = args;
}
const messages = [{ id: "foo", targeting: "foo === 0" }];
const match = await ASRouterTargeting.findMatchingMessage({
messages,
onError,
});
is(
match,
undefined,
"should return nothing since no valid matching message exists"
);
// Note that in order for the following test to pass, we are expecting a particular filepath for mozjexl.
// If the location of this file has changed, the MOZ_JEXL_FILEPATH constant should be updated om ASRouterTargeting.jsm
is(
result[0],
ASRouterTargeting.ERROR_TYPES.MALFORMED_EXPRESSION,
"should recognize the error as coming from mozjexl and call onError with the MALFORMED_EXPRESSION error type"
);
ok(result[1].message, "should call onError with the error from mozjexl");
is(result[2], messages[0], "should call onError with the invalid message");
});
add_task(async function check_other_error_handling() {
let result;
let called = false;
function onError(...args) {
result = args;
called = true;
}
const messages = [{ id: "foo", targeting: "foo" }];
@ -158,19 +103,8 @@ add_task(async function check_other_error_handling() {
undefined,
"should return nothing since no valid matching message exists"
);
// Note that in order for the following test to pass, we are expecting a particular filepath for mozjexl.
// If the location of this file has changed, the MOZ_JEXL_FILEPATH constant should be updated om ASRouterTargeting.jsm
is(
result[0],
ASRouterTargeting.ERROR_TYPES.ATTRIBUTE_ERROR,
"should not recognize the error as being an attribute error."
);
is(
result[1].message,
"test error",
"should call onError with the error thrown in the context"
);
is(result[2], "foo", "should call onError with the invalid attribute");
Assert.ok(called, "Attribute error caught");
});
// ASRouterTargeting.Environment

View File

@ -71,6 +71,7 @@ describe("ASRouter", () => {
let FakeToolbarPanelHub;
let FakeMomentsPageHub;
let personalizedCfrScores;
let fakeTargetingContext;
function createFakeStorage() {
const getStub = sandbox.stub();
@ -168,6 +169,10 @@ describe("ASRouter", () => {
uninit: sandbox.stub(),
executeAction: sandbox.stub(),
};
fakeTargetingContext = {
combineContexts: sandbox.stub(),
evalWithDefault: sandbox.stub().resolves(),
};
globals.set({
// Testing framework doesn't know how to `defineLazyModuleGetter` so we're
// importing these modules into the global scope ourselves.
@ -210,6 +215,15 @@ describe("ASRouter", () => {
SpecialMessageActions: {
handleAction: sandbox.stub(),
},
TargetingContext: class {
static combineContexts(...args) {
return fakeTargetingContext.combineContexts.apply(sandbox, args);
}
evalWithDefault(expr) {
return fakeTargetingContext.evalWithDefault(expr);
}
},
});
await createRouterAndInit();
});
@ -472,7 +486,7 @@ describe("ASRouter", () => {
messages: [messageTargeted, messageNotTargeted],
providers: [{ id: "snippets" }],
});
sandbox.stub(ASRouterTargeting, "isMatch").resolves(false);
fakeTargetingContext.evalWithDefault.resolves(false);
await Router.onPrefChange("services.sync.username");
@ -552,7 +566,7 @@ describe("ASRouter", () => {
beforeEach(async () => {
stub = sandbox.stub();
stub.resolves("foo");
sandbox.stub(ASRouterTargeting, "isMatch").callsFake(stub);
fakeTargetingContext.evalWithDefault.callsFake(stub);
});
afterEach(() => {
sandbox.restore();
@ -3488,9 +3502,7 @@ describe("ASRouter", () => {
sandbox.stub(Router, "loadMessagesFromAllProviders");
});
it("should dispatch an event when a targeting expression throws an error", async () => {
sandbox
.stub(global.FilterExpressions, "eval")
.returns(Promise.reject(new Error("fake error")));
fakeTargetingContext.evalWithDefault.rejects("unit test error");
await Router.setState({
messages: [
{

View File

@ -17,6 +17,7 @@ describe("#CachedTargetingGetter", () => {
let clock;
let frecentStub;
let topsitesCache;
let globals;
beforeEach(() => {
sandbox = sinon.createSandbox();
clock = sinon.useFakeTimers();
@ -26,11 +27,25 @@ describe("#CachedTargetingGetter", () => {
);
sandbox.stub(global.Cu, "reportError");
topsitesCache = new CachedTargetingGetter("getTopFrecentSites");
globals = new GlobalOverrider();
globals.set(
"TargetingContext",
class {
static combineContexts(...args) {
return sinon.stub();
}
evalWithDefault(expr) {
return sinon.stub();
}
}
);
});
afterEach(() => {
sandbox.restore();
clock.restore();
globals.restore();
});
it("should only make a request every 6 hours", async () => {
@ -188,26 +203,6 @@ describe("#CachedTargetingGetter", () => {
assert.equal(arg_m3.id, m2.id);
});
});
describe("combineContexts", () => {
it("should combine the properties of the two objects", () => {
const joined = ASRouterTargeting.combineContexts(
{
get foo() {
return "foo";
},
},
{
get bar() {
return "bar";
},
}
);
const proxy = new Proxy({}, joined);
assert.equal(proxy.foo, "foo");
assert.equal(proxy.bar, "bar");
});
});
});
describe("#CacheListAttachedOAuthClients", () => {
const twoHours = 2 * 60 * 60 * 1000;
@ -262,34 +257,55 @@ describe("ASRouterTargeting", () => {
let evalStub;
let sandbox;
let clock;
let globals;
let fakeTargetingContext;
beforeEach(() => {
sandbox = sinon.createSandbox();
evalStub = sandbox.stub(global.FilterExpressions, "eval");
sandbox.replace(ASRouterTargeting, "Environment", {});
clock = sinon.useFakeTimers();
fakeTargetingContext = {
combineContexts: sandbox.stub(),
evalWithDefault: sandbox.stub().resolves(),
};
globals = new GlobalOverrider();
globals.set(
"TargetingContext",
class {
static combineContexts(...args) {
return fakeTargetingContext.combineContexts.apply(sandbox, args);
}
evalWithDefault(expr) {
return fakeTargetingContext.evalWithDefault(expr);
}
}
);
evalStub = fakeTargetingContext.evalWithDefault;
});
afterEach(() => {
clock.restore();
sandbox.restore();
globals.restore();
});
it("should cache evaluation result", async () => {
evalStub.resolves(true);
let targetingContext = new global.TargetingContext();
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl1" },
{},
targetingContext,
sandbox.stub(),
true
);
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl2" },
{},
targetingContext,
sandbox.stub(),
true
);
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl1" },
{},
targetingContext,
sandbox.stub(),
true
);
@ -298,22 +314,23 @@ describe("ASRouterTargeting", () => {
});
it("should not cache evaluation result", async () => {
evalStub.resolves(true);
let targetingContext = new global.TargetingContext();
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl" },
{},
targetingContext,
sandbox.stub(),
false
);
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl" },
{},
targetingContext,
sandbox.stub(),
false
);
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl" },
{},
targetingContext,
sandbox.stub(),
false
);
@ -322,23 +339,24 @@ describe("ASRouterTargeting", () => {
});
it("should expire cache entries", async () => {
evalStub.resolves(true);
let targetingContext = new global.TargetingContext();
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl" },
{},
targetingContext,
sandbox.stub(),
true
);
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl" },
{},
targetingContext,
sandbox.stub(),
true
);
clock.tick(5 * 60 * 1000 + 1);
await ASRouterTargeting.checkMessageTargeting(
{ targeting: "jexl" },
{},
targetingContext,
sandbox.stub(),
true
);

View File

@ -6,3 +6,4 @@ toolkit.jar:
% resource messaging-system %res/messaging-system/
res/messaging-system/experiments/ (./experiments/*)
res/messaging-system/lib/ (./lib/*)
res/messaging-system/targeting/ (./targeting/*)

View File

@ -20,6 +20,7 @@ const { XPCOMUtils } = ChromeUtils.import(
XPCOMUtils.defineLazyModuleGetters(this, {
ASRouterTargeting: "resource://activity-stream/lib/ASRouterTargeting.jsm",
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
ExperimentManager:
"resource://messaging-system/experiments/ExperimentManager.jsm",
RemoteSettings: "resource://services-settings/remote-settings.js",
@ -106,20 +107,24 @@ class _RemoteSettingsExperimentLoader {
* @returns {Promise<boolean>} Should we process the recipe?
*/
async checkTargeting(recipe, customContext = {}) {
const context = TargetingContext.combineContexts(
customContext,
ASRouterTargeting.Environment
);
const { targeting } = recipe;
if (!targeting) {
log.debug("No targeting for recipe, so it matches automatically");
return true;
}
log.debug("Testing targeting expression:", targeting);
const result = await ASRouterTargeting.isMatch(
targeting,
customContext,
err => {
log.debug("Targeting failed because of an error");
Cu.reportError(err);
}
);
const targetingContext = new TargetingContext(context);
let result = false;
try {
result = await targetingContext.evalWithDefault(targeting);
} catch (e) {
log.debug("Targeting failed because of an error");
Cu.reportError(e);
}
return Boolean(result);
}

View File

@ -15,6 +15,7 @@ BROWSER_CHROME_MANIFESTS += [
SPHINX_TREES['docs'] = 'schemas'
XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
XPCSHELL_TESTS_MANIFESTS += ['targeting/test/unit/xpcshell.ini']
TESTING_JS_MODULES += [
'schemas/SpecialMessageActionSchemas/SpecialMessageActionSchemas.json',

View File

@ -0,0 +1,203 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
XPCOMUtils.defineLazyModuleGetters(this, {
Services: "resource://gre/modules/Services.jsm",
clearTimeout: "resource://gre/modules/Timer.jsm",
setTimeout: "resource://gre/modules/Timer.jsm",
ASRouterTargeting: "resource://activity-stream/lib/ASRouterTargeting.jsm",
FilterExpressions:
"resource://gre/modules/components-utils/FilterExpressions.jsm",
ClientEnvironment: "resource://normandy/lib/ClientEnvironment.jsm",
});
var EXPORTED_SYMBOLS = ["TargetingContext"];
const TARGETING_EVENT_CATEGORY = "messaging_experiments";
const TARGETING_EVENT_METHOD = "targeting";
const DEFAULT_TIMEOUT = 3000;
const ERROR_TYPES = {
ATTRIBUTE_ERROR: "attribute_error",
TIMEOUT: "attribute_timeout",
};
const TargetingEnvironment = {
get locale() {
return ASRouterTargeting.Environment.locale;
},
get localeLanguageCode() {
return ASRouterTargeting.Environment.localeLanguageCode;
},
get region() {
return ASRouterTargeting.Environment.region;
},
get userId() {
return ClientEnvironment.userId;
},
};
class TargetingContext {
constructor(customContext) {
if (customContext) {
this.ctx = new Proxy(customContext, {
get: (customCtx, prop) => {
if (prop in TargetingEnvironment) {
return TargetingEnvironment[prop];
}
return customCtx[prop];
},
});
} else {
this.ctx = TargetingEnvironment;
}
// Enable event recording
Services.telemetry.setEventRecordingEnabled(TARGETING_EVENT_CATEGORY, true);
}
_sendUndesiredEvent(eventData) {
Services.telemetry.recordEvent(
TARGETING_EVENT_CATEGORY,
TARGETING_EVENT_METHOD,
eventData.event,
eventData.value
);
}
/**
* Wrap each property of context[key] with a Proxy that captures errors and
* timeouts
*
* @param {Object.<string, TargetingGetters> | TargetingGetters} context
* @param {string} key Namespace value found in `context` param
* @returns {TargetingGetters} Wrapped context where getter report errors and timeouts
*/
createContextWithTimeout(context, key = null) {
const timeoutDuration = key ? context[key].timeout : context.timeout;
const logUndesiredEvent = (event, key, prop) => {
const value = key ? `${key}.${prop}` : prop;
this._sendUndesiredEvent({ event, value });
Cu.reportError(`${event}: ${value}`);
};
return new Proxy(context, {
get(target, prop) {
// eslint-disable-next-line no-async-promise-executor
return new Promise(async (resolve, reject) => {
// Create timeout cb to record attribute resolution taking too long.
let timeout = setTimeout(() => {
logUndesiredEvent(ERROR_TYPES.TIMEOUT, key, prop);
reject(
new Error(
`${prop} targeting getter timed out after ${timeoutDuration ||
DEFAULT_TIMEOUT}ms`
)
);
}, timeoutDuration || DEFAULT_TIMEOUT);
try {
resolve(await (key ? target[key][prop] : target[prop]));
} catch (error) {
logUndesiredEvent(ERROR_TYPES.ATTRIBUTE_ERROR, key, prop);
reject(error);
Cu.reportError(error);
} finally {
clearTimeout(timeout);
}
});
},
});
}
/**
* Merge all evaluation contexts and wrap the getters with timeouts
*
* @param {Object.<string, TargetingGetters>[]} contexts
* @returns {Object.<string, TargetingGetters>} Object that follows the pattern of `namespace: getters`
*/
mergeEvaluationContexts(contexts) {
let context = {};
for (let c of contexts) {
for (let envNamespace of Object.keys(c)) {
// Take the provided context apart, replace it with a proxy
context[envNamespace] = this.createContextWithTimeout(c, envNamespace);
}
}
return context;
}
/**
* Merge multiple TargetingGetters objects without accidentally evaluating
*
* @param {TargetingGetters[]} ...contexts
* @returns {Proxy<TargetingGetters>}
*/
static combineContexts(...contexts) {
return new Proxy(
{},
{
get(target, prop) {
for (let context of contexts) {
if (prop in context) {
return context[prop];
}
}
return null;
},
}
);
}
/**
* Evaluate JEXL expressions with default `TargetingEnvironment` and custom
* provided targeting contexts
*
* @example
* eval(
* "ctx.locale == 'en-US' && customCtx.foo == 42",
* { customCtx: { foo: 42 } }
* ); // true
*
* @param {string} expression JEXL expression
* @param {Object.<string, TargetingGetters>[]} ...contexts Additional custom context
* objects where the keys act as namespaces for the different getters
*
* @returns {promise} Evaluation result
*/
eval(expression, ...contexts) {
return FilterExpressions.eval(
expression,
this.mergeEvaluationContexts([{ ctx: this.ctx }, ...contexts])
);
}
/**
* Evaluate JEXL expressions with default provided targeting context
*
* @example
* new TargetingContext({ bar: 42 });
* evalWithDefault(
* "bar == 42",
* ); // true
*
* @param {string} expression JEXL expression
* @returns {promise} Evaluation result
*/
evalWithDefault(expression) {
return FilterExpressions.eval(
expression,
this.createContextWithTimeout(this.ctx)
);
}
}

View File

@ -0,0 +1,5 @@
"use strict";
// Globals
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");

View File

@ -0,0 +1,206 @@
const { TargetingContext } = ChromeUtils.import(
"resource://messaging-system/targeting/Targeting.jsm"
);
const { TelemetryTestUtils } = ChromeUtils.import(
"resource://testing-common/TelemetryTestUtils.jsm"
);
add_task(async function instance_with_default() {
let targeting = new TargetingContext();
let res = await targeting.eval(
`ctx.locale == '${Services.locale.appLocaleAsBCP47}'`
);
Assert.ok(res, "Has local context");
});
add_task(async function instance_with_context() {
let targeting = new TargetingContext({ bar: 42 });
let res = await targeting.eval("ctx.bar == 42");
Assert.ok(res, "Merge provided context with default");
});
add_task(async function eval_1_context() {
let targeting = new TargetingContext();
let res = await targeting.eval("custom1.bar == 42", { custom1: { bar: 42 } });
Assert.ok(res, "Eval uses provided context");
});
add_task(async function eval_2_context() {
let targeting = new TargetingContext();
let res = await targeting.eval("custom1.bar == 42 && custom2.foo == 42", {
custom1: { bar: 42 },
custom2: { foo: 42 },
});
Assert.ok(res, "Eval uses provided context");
});
add_task(async function eval_multiple_context() {
let targeting = new TargetingContext();
let res = await targeting.eval(
"custom1.bar == 42 && custom2.foo == 42 && custom3.baz == 42",
{ custom1: { bar: 42 }, custom2: { foo: 42 } },
{ custom3: { baz: 42 } }
);
Assert.ok(res, "Eval uses provided context");
});
add_task(async function eval_multiple_context_precedence() {
let targeting = new TargetingContext();
let res = await targeting.eval(
"custom1.bar == 42 && custom2.foo == 42",
{ custom1: { bar: 24 }, custom2: { foo: 24 } },
{ custom1: { bar: 42 }, custom2: { foo: 42 } }
);
Assert.ok(res, "Last provided context overrides previously defined ones.");
});
add_task(async function eval_evalWithDefault() {
let targeting = new TargetingContext({ foo: 42 });
let res = await targeting.evalWithDefault("foo == 42");
Assert.ok(res, "Eval uses provided context");
});
add_task(async function log_targeting_error_events() {
let ctx = {
get foo() {
throw new Error("unit test");
},
};
let targeting = new TargetingContext(ctx);
let stub = sinon.stub(targeting, "_sendUndesiredEvent");
await Assert.rejects(
targeting.evalWithDefault("foo == 42", ctx),
/unit test/,
"Getter should throw"
);
Assert.equal(stub.callCount, 1, "Error event was logged");
let {
args: [{ event, value }],
} = stub.firstCall;
Assert.equal(event, "attribute_error", "Correct error message");
Assert.equal(value, "foo", "Correct attribute name");
});
add_task(async function eval_evalWithDefault_precedence() {
let targeting = new TargetingContext({ region: "space" });
let res = await targeting.evalWithDefault("region != 'space'");
Assert.ok(res, "Custom context does not override TargetingEnvironment");
});
add_task(async function eval_evalWithDefault_combineContexts() {
let combinedCtxs = TargetingContext.combineContexts({ foo: 1 }, { foo: 2 });
let targeting = new TargetingContext(combinedCtxs);
let res = await targeting.evalWithDefault("foo == 1");
Assert.ok(res, "First match is returned for combineContexts");
});
add_task(async function log_targeting_error_events_in_namespace() {
let ctx = {
get foo() {
throw new Error("unit test");
},
};
let targeting = new TargetingContext(ctx);
let stub = sinon.stub(targeting, "_sendUndesiredEvent");
let catchStub = sinon.stub();
try {
await targeting.eval("ctx.foo == 42");
} catch (e) {
catchStub();
}
Assert.equal(stub.callCount, 1, "Error event was logged");
let {
args: [{ event, value }],
} = stub.firstCall;
Assert.equal(event, "attribute_error", "Correct error message");
Assert.equal(value, "ctx.foo", "Correct attribute name");
Assert.ok(catchStub.calledOnce, "eval throws errors");
});
add_task(async function log_timeout_errors() {
let ctx = {
timeout: 1,
get foo() {
return new Promise(() => {});
},
};
let targeting = new TargetingContext(ctx);
let stub = sinon.stub(targeting, "_sendUndesiredEvent");
let catchStub = sinon.stub();
try {
await targeting.eval("ctx.foo");
} catch (e) {
catchStub();
}
Assert.equal(catchStub.callCount, 1, "Timeout error throws");
Assert.equal(stub.callCount, 1, "Timeout event was logged");
let {
args: [{ event, value }],
} = stub.firstCall;
Assert.equal(event, "attribute_timeout", "Correct error message");
Assert.equal(value, "ctx.foo", "Correct attribute name");
});
add_task(async function test_telemetry_event_timeout() {
Services.telemetry.clearEvents();
let ctx = {
timeout: 1,
get foo() {
return new Promise(() => {});
},
};
let expectedEvents = [
["messaging_experiments", "targeting", "attribute_timeout", "ctx.foo"],
];
let targeting = new TargetingContext(ctx);
try {
await targeting.eval("ctx.foo");
} catch (e) {}
TelemetryTestUtils.assertEvents(expectedEvents);
Services.telemetry.clearEvents();
});
add_task(async function test_telemetry_event_error() {
Services.telemetry.clearEvents();
let ctx = {
get bar() {
throw new Error("unit test");
},
};
let expectedEvents = [
["messaging_experiments", "targeting", "attribute_error", "ctx.bar"],
];
let targeting = new TargetingContext(ctx);
try {
await targeting.eval("ctx.bar");
} catch (e) {}
TelemetryTestUtils.assertEvents(expectedEvents);
Services.telemetry.clearEvents();
});

View File

@ -0,0 +1,6 @@
[DEFAULT]
head = head.js
tags = messaging-system
firefox-appdir = browser
[test_targeting.js]

View File

@ -24,6 +24,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
Storage: "resource://normandy/lib/Storage.jsm",
FilterExpressions:
"resource://gre/modules/components-utils/FilterExpressions.jsm",
TargetingContext: "resource://messaging-system/targeting/Targeting.jsm",
NormandyApi: "resource://normandy/lib/NormandyApi.jsm",
ClientEnvironment: "resource://normandy/lib/ClientEnvironment.jsm",
CleanupManager: "resource://normandy/lib/CleanupManager.jsm",
@ -539,8 +540,9 @@ var RecipeRunner = {
}
const context = this.getFilterContext(recipe);
const targetingContext = new TargetingContext();
try {
if (await FilterExpressions.eval(recipe.filter_expression, context)) {
if (await targetingContext.eval(recipe.filter_expression, context)) {
yield BaseAction.suitability.FILTER_MATCH;
} else {
yield BaseAction.suitability.FILTER_MISMATCH;

View File

@ -922,6 +922,26 @@ messaging_experiments:
branches: >
A semicolon separated string for all the qualified branch ID(s).
e.g. "control;variant_01;treatment_02".
targeting:
objects:
- attribute_error
- attribute_timeout
methods:
- targeting
release_channel_collection: opt-out
products:
- firefox
record_in_processes:
- main
description: >
Record generic JEXL errors that result from issues with experiment or
message targeting expressions. The value field contains the namespace and
attribute name that caused the error.
bug_numbers:
- 1644743
notification_emails:
- ujet@mozilla.com
expiry_version: never
# This category contains event entries used for Telemetry tests.
# They will not be sent out with any pings.