Closed Bug 1351465 Opened 3 years ago Closed 3 years ago

Exception running bootstrap method shutdown on shield-recipe-client@mozilla.org: TypeError: log is null

Categories

(Firefox :: Normandy Client, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: mythmon, Assigned: mythmon)

Details

Attachments

(1 file)

https://treeherder.mozilla.org/logviewer.html#?job_id=86367475&repo=autoland&lineNumber=1622

It looks like there are cases where bootstrap::shutdown is either called multiple times, or is called without bootstrap::startup being called.
(In reply to Mike Cooper [:mythmon] from comment #0)
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=86367475&repo=autoland&lineNumber=1622
> 
> It looks like there are cases where bootstrap::shutdown is either called
> multiple times, or is called without bootstrap::startup being called.

Are you logging the bootstrap reason? It's the second param passed to the bootstrap methods like startup and shutdown.
Flags: needinfo?(mcooper)
We aren't logging the bootstrap reason. I wrote a quick patch to do that, and ran a try build with it. Hopefully it will happen again so we can see what is going on.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=895ff27280d41abd68a7179b85acfead6f57350b
Flags: needinfo?(mcooper)
(In reply to Mike Cooper [:mythmon] from comment #2)
> We aren't logging the bootstrap reason. I wrote a quick patch to do that,
> and ran a try build with it. Hopefully it will happen again so we can see
> what is going on.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=895ff27280d41abd68a7179b85acfead6f57350b

It happened again but I see no extra logging at all. I saw this locally yesterday though, when running some tests - from terminal history, looks like I ran ./mach test toolkit/components/perfmonitoring/ on my mac - can you not reproduce locally?
Flags: needinfo?(mcooper)
I was able to reproduce this locally with ./mach run toolkit/components/perfmonitoring. I changed the logging to use dump, and found out that my initial diagnosis was wrong, startup and shutdown are being called exactly as expected. The real issue is that in some cases, including these tests, the startup function returns early, and doesn't set up the logging. When the shutdown function tries to use logging, it hasn't been set up, causing this error. The fix was to set up logging before returning early.
Flags: needinfo?(mcooper)
Comment on attachment 8852662 [details]
Bug 1351465 - shield-recipe-client: Unconditionally set up logging in bootstrap

https://reviewboard.mozilla.org/r/124846/#review127618
Attachment #8852662 - Flags: review?(gijskruitbosch+bugs) → review+
Assignee: nobody → mcooper
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a381650bea90
shield-recipe-client: Unconditionally set up logging in bootstrap r=Gijs
https://hg.mozilla.org/mozilla-central/rev/a381650bea90
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Commits pushed to master at https://github.com/mozilla/normandy

https://github.com/mozilla/normandy/commit/60a3fa10398b0327797e2e58508373f4f74fc732
Bug 1351465 - recipe-client-addon: Unconditionally set up logging in bootstrap

https://github.com/mozilla/normandy/commit/5513601f46672368fa45545346cf7b5e795e5220
Merge pull request #649 from mythmon/log-is-null

Bug 1351465 - recipe-client-addon: Unconditionally set up logging in bootstrap
Can we uplift this to Aurora now too since bug 1349348 just landed there?
Flags: needinfo?(mcooper)
Comment on attachment 8852662 [details]
Bug 1351465 - shield-recipe-client: Unconditionally set up logging in bootstrap

Approval Request Comment
[Feature/Bug causing the regression]: bug 1349348
[User impact if declined]: Errors in browser console on shutdown
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: This is a well verified, simple change
[String changes made/needed]: None
Flags: needinfo?(mcooper)
Attachment #8852662 - Flags: approval-mozilla-aurora?
Comment on attachment 8852662 [details]
Bug 1351465 - shield-recipe-client: Unconditionally set up logging in bootstrap

Since bug 1349348 was landed in aurora. Let's also take this. Aurora54+.
Attachment #8852662 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.