773892 - Implement minimal (headless) crash submitter for gonk

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[(not currently active) Ted Mielczarek]

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

[Robert Kaiser]

I believe having crash reporting working for B2G should block basecamp.

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

Hub agreed to jump on this :)

Ted I lost my IRC log - can you reiterate what this bug basically requires?

Comment 3

[(not currently active) Ted Mielczarek]

It should all be summarized in comment 0.

Comment 4

[Hubert Figuiere [:hub]]

Attachment: Bug 773892 - Add crash submission when starting b2g.

Here is my proposed fix. Requesting feedback to see if that's what you envisioned.

Comment 5

[(not currently active) Ted Mielczarek]

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.

Comment 6

[Hubert Figuiere [:hub]]

Attachment: 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)

Comment 7

[Hubert Figuiere [:hub]]

Crash Reports are left in "pending" because it seems that there is no network access.

Comment 8

[(not currently active) Ted Mielczarek]

We need to make sure that works. Can you add a listener for network state and send when the network is online?

Comment 9

[Hubert Figuiere [:hub]]

I don't seem to be able to get the "online" event from the window. My event listener is never called.

Comment 10

[Philipp von Weitershausen [:philikon]]

(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.

Comment 11

[(not currently active) Ted Mielczarek]

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).

Comment 12

[Hubert Figuiere [:hub]]

(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.

Comment 13

[Hubert Figuiere [:hub]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64d7f92c3029

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/64d7f92c3029

Comment 15

[Hubert Figuiere [:hub]]

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

Comment 16

[(not currently active) Ted Mielczarek]

I don't think that's harmful.

Comment 17

[Robert Kaiser]

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. ;-)

Comment 18

[Hubert Figuiere [:hub]]

I don't see it all the time either. :-)

Comment 19

[Hubert Figuiere [:hub]]

Attachment: Bug 773892 - Part 2: Check for network status and delay until reconnect.

Need the blocking bugs to be fixed to work.

Comment 20

[(not currently active) Ted Mielczarek]

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.

Comment 21

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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.

Comment 22

[Hubert Figuiere [:hub]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0bdc5212ba83

Comment 23

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0bdc5212ba83

Comment 24

[Robert Kaiser]

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?)

Comment 25

[Hubert Figuiere [:hub]]

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

Comment 26

[Robert Kaiser]

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).

Comment 27

[Hubert Figuiere [:hub]]

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

[Robert Kaiser]

(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

[Robert Kaiser]

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?

Comment 30

[Hubert Figuiere [:hub]]

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.

Comment 31

[Hubert Figuiere [:hub]]

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

Comment 32

[Hubert Figuiere [:hub]]

reopening

Comment 33

[Hubert Figuiere [:hub]]

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).

Comment 34

[Hubert Figuiere [:hub]]

Attachment: Bug 773892 - Part 3: only rely on the event to send the crash report.

Comment 35

[Robert Kaiser]

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.

Comment 36

[Hubert Figuiere [:hub]]

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 !

Comment 37

[Hubert Figuiere [:hub]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2644a3626ec

Comment 38

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b2644a3626ec