Closed Bug 1331705 Opened 6 years ago Closed 6 years ago

Do not use an XRay-ed Promise during recipe execution

Categories

(Firefox :: Normandy Client, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mythmon, Assigned: mythmon)

Details

Attachments

(1 file)

This comes from https://github.com/mozilla/normandy-addon/pull/43

From that PR:

> The RecipeRunner uses a promise from the sandbox to detect when
> execution is done, but the promise methods aren't available since the
> object is XRay-ed and its prototype is set to Object.
> 
> Instead, this patch uses two new functions passed to the sandbox that
> map to resolving or rejecting a promise. The new executeAction function
> returns a Promise defined outside the sandbox, but resolved or rejected
> from within it when execution is finished.
> 
> The Promise also makes testing a bit easier as we can now inspect the
> value that the sandbox resolves with.
Gijs: You reviewed this in GitHub, but I wasn't sure if I should tag it for review in a bug before landing it, so I'm erring on the side of more review.
Assignee: nobody → mcooper
Comment on attachment 8827581 [details]
Bug 1331705 - shield-recipe-client: Do not use an XRay-ed Promise during recipe execution,

https://reviewboard.mozilla.org/r/105210/#review106108

Yeah, I think you can just land things if I r+'d on github. See also some more detail on my thoughts about this in https://github.com/mozilla/normandy/pull/428#issuecomment-273334894 .

(I'm just assuming this is a straight port and not looking at this again. :-)
Attachment #8827581 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ce90a9d52e86
shield-recipe-client: Do not use an XRay-ed Promise during recipe execution, r=Gijs
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a506d21c911e
shield-recipe-client: Do not use an XRay-ed Promise during recipe execution, r=Gijs
https://hg.mozilla.org/mozilla-central/rev/a506d21c911e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Flags: needinfo?(mcooper)
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.