Implement minimal (headless) crash submitter for gonk

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ted, Assigned: hub)

Tracking

(Blocks: 1 bug)

unspecified
Dependency tree / graph

Firefox Tracking Flags

(blocking-kilimanjaro:+, blocking-basecamp:+)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Split this off from bug 761909. That bug implements just the plumbing for crash reporting. minidumps will get written, but no crashreporter will be launched on crash. On restart, the last crash ID will be exposeod as nsIXULRuntime.lastCrashID, which chrome code can pass to CrashSubmit.jsm's submit method. This bug covers that last bit, hooking up the submission in the chrome code. Per a previous discussion we also want to have a pref somewhere to control automatic submission.

Comment 1

5 years ago
I believe having crash reporting working for B2G should block basecamp.
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
blocking-kilimanjaro: --- → +

Updated

5 years ago
Blocks: 774290
Hub agreed to jump on this :)

Ted I lost my IRC log - can you reiterate what this bug basically requires?
Assignee: nobody → hub
(Reporter)

Comment 3

5 years ago
It should all be summarized in comment 0.
(Assignee)

Comment 4

5 years ago
Created attachment 643665 [details] [diff] [review]
Bug 773892 - Add crash submission when starting b2g.

Here is my proposed fix. Requesting feedback to see if that's what you envisioned.
Attachment #643665 - Flags: feedback?(ted.mielczarek)
(Reporter)

Comment 5

5 years ago
Comment on attachment 643665 [details] [diff] [review]
Bug 773892 - Add crash submission when starting b2g.

Conceptually this is doing all the right things. Making it happen at the right *time* is the hard part.
Attachment #643665 - Flags: feedback?(ted.mielczarek) → feedback+
(Assignee)

Comment 6

5 years ago
Created attachment 644970 [details] [diff] [review]
Bug 773892 - Add crash submission when starting b2g.

Updated and test patch. Reporting is moved to the event loop in the startup phase.

(needs bug 761909)
Attachment #643665 - Attachment is obsolete: true
Attachment #644970 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 7

5 years ago
Crash Reports are left in "pending" because it seems that there is no network access.
(Reporter)

Comment 8

5 years ago
We need to make sure that works. Can you add a listener for network state and send when the network is online?
(Assignee)

Comment 9

5 years ago
I don't seem to be able to get the "online" event from the window. My event listener is never called.
(In reply to Hub Figuiere [:hub] from comment #9)
> I don't seem to be able to get the "online" event from the window. My event
> listener is never called.

It's true, we don't currently update nsIIOService::offline (which I believe controls navigator.onLine and the online/offline events). I filed bug 777145 for this. Note that if you're in chrome code (this includes shell.js), it's probably better to just use nsIIOService (Services.io) and/or the observer notifications.
(Assignee)

Updated

5 years ago
Depends on: 777145
(Reporter)

Comment 11

5 years ago
Comment on attachment 644970 [details] [diff] [review]
Bug 773892 - Add crash submission when starting b2g.

Review of attachment 644970 [details] [diff] [review]:
-----------------------------------------------------------------

This code looks fine, but I think we probably still want to observe the network status and only try to submit if we're online. Also, we may want to consider only sending when on WiFi (although minidumps are fairly small, usually only ~100KB).
Attachment #644970 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 12

5 years ago
(In reply to Ted Mielczarek [:ted] from comment #11)
> This code looks fine, but I think we probably still want to observe the
> network status and only try to submit if we're online. Also, we may want to
> consider only sending when on WiFi (although minidumps are fairly small,
> usually only ~100KB).

I'll make this part 2. There are a couple of blockers on the platform that needs to be ironed out. And yes I was wondering if we should only do it over wifi. Will add that to part 2.
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/64d7f92c3029
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/64d7f92c3029
(Assignee)

Comment 15

5 years ago
ted, I'm getting this on the console when actually submitting:

E/GeckoConsole( 4369): crash-reports.mozilla.com : server does not support RFC 5746, see CVE-2009-3555
(Reporter)

Comment 16

5 years ago
I don't think that's harmful.

Comment 17

5 years ago
Right, I've seen this warning for quite a few servers, it's just a warning, the connection (and therefore submission) works without problems. Actually, you could see that log message as a confirmation that the live server was actually contacted. ;-)
(Assignee)

Comment 18

5 years ago
I don't see it all the time either. :-)
(Assignee)

Comment 19

5 years ago
Created attachment 647964 [details] [diff] [review]
Bug 773892 - Part 2: Check for network status and delay until reconnect.

Need the blocking bugs to be fixed to work.
Attachment #647964 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 20

5 years ago
Comment on attachment 647964 [details] [diff] [review]
Bug 773892 - Part 2: Check for network status and delay until reconnect.

You probably just want review from a B2G peer on this.
Attachment #647964 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #647964 - Flags: review?(21)
Comment on attachment 647964 [details] [diff] [review]
Bug 773892 - Part 2: Check for network status and delay until reconnect.

Review of attachment 647964 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/chrome/content/shell.js
@@ +89,5 @@
>          crashID) {
> +
> +      if (!Services.io.offline) {
> +        this.CrashSubmit.submit(crashID);
> +      } else {

Do an early return.

@@ +90,5 @@
> +
> +      if (!Services.io.offline) {
> +        this.CrashSubmit.submit(crashID);
> +      } else {
> +        Services.obs.addObserver(function observer(aSubject, aTopic, aState) {

nit: subject, topic, state

This file does not prefix arguments.
Attachment #647964 - Flags: review?(21) → review+
(Assignee)

Comment 22

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bdc5212ba83
https://hg.mozilla.org/mozilla-central/rev/0bdc5212ba83

Comment 24

5 years ago
Hub, does the dependency landing mean that this bug can be resolved fixed? (I know you have trouble testing at least some parts with your device because of bug 778360, but should it work for other devices like otoro now?)
(Assignee)

Comment 25

5 years ago
It should work for Otoro, so we can resolve it. The other fix is coming as well anyway.

Comment 26

5 years ago
OK, marking this way per Hub's comment #25. Thanks a ton for this work!

I'd love someone with an Otoro to test (not me yet, but hopefully soon) and would love to see a Gecko crash report submitted from a device. If that works, we can go pushing forward on the other needed parts (app crashes, etc).
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
(Assignee)

Comment 27

5 years ago
Don't forget to check https://bugzilla.mozilla.org/show_bug.cgi?id=761909#c13 before testing. Otherwise you won't have crash reporting.

Comment 28

5 years ago
(In reply to Hub Figuiere [:hub] from comment #27)
> Don't forget to check
> https://bugzilla.mozilla.org/show_bug.cgi?id=761909#c13 before testing.
> Otherwise you won't have crash reporting.

Thanks, we need to add that into the mozconfigs for the Nightly builds as well, I mentioned that in bug 785698 now, which already has other releng changes we need.

Comment 29

5 years ago
Even though this and all dependencies are fixed now, this doesn't seem to work, the reports keep going to "pending" only and not being submitted. Hub, does it work on your device? Should we reopen here or file a new bug?
(Assignee)

Comment 30

5 years ago
It has worked on my device in the past - but lot of things changed since. 

Indeed they seem to be stuck now.

I'll have a look.
(Assignee)

Comment 31

5 years ago
There seems to be something time dependent in the crash submission. Adding some Services.console.logStringMessage() and al make it work.
(Assignee)

Comment 32

5 years ago
reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 33

5 years ago
If Services.io.offline is false, we try to submit the crash report immediately. In that case it fails. Tracing it show that Submitter.prototype.submitForm() xhr.status is 0 (we expect 200 on success).
(Assignee)

Comment 34

5 years ago
Created attachment 664757 [details] [diff] [review]
Bug 773892 - Part 3: only rely on the event to send the crash report.
Attachment #664757 - Flags: review?(kairo)

Comment 35

5 years ago
Comment on attachment 664757 [details] [diff] [review]
Bug 773892 - Part 3: only rely on the event to send the crash report.

From what you told me on IRC, this looks good to me, but I technically don't think I can review B2G or gaia patches, so I'll defer to Vivien, given he has reviewed the previous patch here as well.
Attachment #664757 - Flags: review?(kairo)
Attachment #664757 - Flags: review?(21)
Attachment #664757 - Flags: feedback+
(Assignee)

Updated

5 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 36

5 years ago
Comment on attachment 664757 [details] [diff] [review]
Bug 773892 - Part 3: only rely on the event to send the crash report.

maybe fabrice can review this. Thanks !
Attachment #664757 - Flags: review?(21) → review?(fabrice)
Attachment #664757 - Flags: review?(fabrice) → review+
(Assignee)

Comment 37

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2644a3626ec
https://hg.mozilla.org/mozilla-central/rev/b2644a3626ec
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.