1488523 - Annotate crash reports if web replay is running

Status: RESOLVED FIXED

(Core Graveyard :: Web Replay, enhancement)

Description

[Andrew McCreight [:mccr8]]

It won't always be apparent if a crash happened in a process that is recording or replaying. By adding an annotation if we are recording/replaying, maybe it will be easier to tell when a new crash only happens then. (Of course, crashes that happen normally can also happen during recording/replaying.)

Comment 1

[Brian Hackett [Laid off!]]

Attachment: Part 1 - Add RecordReplay crash annotation.

Add a RecordReplay crash annotation that is always set for reports in middleman or recording/replaying processes.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: Part 2 - Don't drop delayed crash annotations at startup.

By itself, the patch in part 1 doesn't do anything.  The annotation is added before the crash reporter client is initialized, so a delayed note is generated instead.  Unfortunately, at this point in startup the delayed notes have already been processed.  The basic problem seems to be that the delayed notes are processed when the remote exception handler is initialized, when they are actually contingent on the crash reporter client existing.

Comment 3

[Andrew McCreight [:mccr8]]

You'll also want to file a bug against Soccoro that will make it so that the value is publicly queryable and can be used as a column in searches, etc. I'm not sure on the specifics, but somebody who works on that stuff should be able to figure it out.

Comment 4

[Gabriele Svelto [:gsvelto]]

Comment on attachment 9006666 [details] [diff] [review]
Part 1 - Add RecordReplay crash annotation.

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

::: toolkit/crashreporter/CrashAnnotations.yaml
@@ +579,5 @@
>    type: string
>  
> +RecordReplay:
> +  description: >
> +    Set to "true" if this crash happened in a Web Replay middleman, recording,

nit: booleans in crash annotations translate to 1 for true and 0 or absent for false, while this is a little awkward it's better to stick to the existing convention. Change this bit of the description from "true" to 1 like in other boolean entries.

Comment 5

[Gabriele Svelto [:gsvelto]]

Comment on attachment 9006667 [details] [diff] [review]
Part 2 - Don't drop delayed crash annotations at startup.

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

Ouch, I never noticed this was the case, thanks for fixing this.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/65a6efaed937
Part 1 - Add RecordReplay crash annotation, r=gsvelto.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5663441ffe0
Part 2 - Don't drop delayed crash annotations at startup, r=gsvelto.

Comment 7

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/65a6efaed937
https://hg.mozilla.org/mozilla-central/rev/d5663441ffe0