444103 - add ability to add string-formatted notes to crash report

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Josh Aas]

We should add the ability to add string-formatted notes to crash report. This could be used, for example, to store the information contained in an exception object info field to help us fix crashes that come from exceptions.

This would be a sort of catch-all field of textual data that the server stores and displays via socorro. This would probably involve adding a method to the nsICrashReporter interface that is like the existing annotateReport.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: add appendAppNotesToCrashReport method to nsICrashReporter interface+impl

This adds a new method to nsICrashReporter, appendAppNotesToCrashReport(in ACString data). It appends whatever you pass it to a single string, and adds it to the crash report in a field named "Notes".

I also reworked the unit tests a little bit while adding a test for this method.

Comment 2

[Benjamin Smedberg]

Comment on attachment 331176 [details] [diff] [review]
add appendAppNotesToCrashReport method to nsICrashReporter interface+impl

You need to rev the IID of nsICrashReporter before final push.

Comment 3

[(not currently active) Ted Mielczarek]

rev'ed the IID and pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/61c0a4ca72e4

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 331176 [details] [diff] [review]
add appendAppNotesToCrashReport method to nsICrashReporter interface+impl

Requesting approval for 1.9.0.x for this patch, since it (in conjunction with bug 451709) would really help decipher the extremely high number of Mac crashes that are lumped under nsObjCExceptionLogAbort(NSException*).

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 331176 [details] [diff] [review]
add appendAppNotesToCrashReport method to nsICrashReporter interface+impl

Better to get this early next time than late this time.

Comment 6

[Samuel Sidler (old account; do not CC)]

Ted, can you provide a new patch for this that doesn't change the existing interface?

Comment 7

[Samuel Sidler (old account; do not CC)]

Ted, any update here?

Comment 8

[(not currently active) Ted Mielczarek]

Sorry, I've been really busy on other things. I can do a branch patch, I just need to find some time.

Comment 9

[(not currently active) Ted Mielczarek]

Attachment: rollup branch patch for this bug + bug 451709

Ok, this is a branch patch including the patch here as well as the patch from bug 451709. Added a new interface with the added methods and changed the iface references in the code. Works fine, I have an updated CrashMe extension that can be used to test, I'll post directions shortly.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 348212 [details] [diff] [review]
rollup branch patch for this bug + bug 451709

often it makes more sense to inherit from the original interface rather than a parallel unrelated _BRANCH interface.

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 11

[Daniel Veditz [:dveditz]]

please add the "fixed1.9.0.5" to bug 451709 as well when you check this in so QA remembers to verify that part of the fix.

Comment 12

[(not currently active) Ted Mielczarek]

Will do. Going to try to add a litmus test to that bug as well.

Checking in toolkit/crashreporter/mac_utils.h;
/cvsroot/mozilla/toolkit/crashreporter/mac_utils.h,v  <--  mac_utils.h
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/crashreporter/mac_utils.mm;
/cvsroot/mozilla/toolkit/crashreporter/mac_utils.mm,v  <--  mac_utils.mm
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/crashreporter/nsExceptionHandler.cpp;
/cvsroot/mozilla/toolkit/crashreporter/nsExceptionHandler.cpp,v  <--  nsExceptionHandler.cpp
new revision: 1.39; previous revision: 1.38
done
Checking in toolkit/crashreporter/nsExceptionHandler.h;
/cvsroot/mozilla/toolkit/crashreporter/nsExceptionHandler.h,v  <--  nsExceptionHandler.h
new revision: 1.9; previous revision: 1.8
done
Checking in toolkit/crashreporter/test/TestCrashReporterAPI.cpp;
/cvsroot/mozilla/toolkit/crashreporter/test/TestCrashReporterAPI.cpp,v  <--  TestCrashReporterAPI.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/xre/MacLaunchHelper.h;
/cvsroot/mozilla/toolkit/xre/MacLaunchHelper.h,v  <--  MacLaunchHelper.h
new revision: 1.6; previous revision: 1.5
done
Removing toolkit/xre/MacLaunchHelper.m;
/cvsroot/mozilla/toolkit/xre/MacLaunchHelper.m,v  <--  MacLaunchHelper.m
new revision: delete; previous revision: 1.9
done
RCS file: /cvsroot/mozilla/toolkit/xre/MacLaunchHelper.mm,v
done
Checking in toolkit/xre/MacLaunchHelper.mm;
/cvsroot/mozilla/toolkit/xre/MacLaunchHelper.mm,v  <--  MacLaunchHelper.mm
initial revision: 1.1
done
Checking in toolkit/xre/Makefile.in;
/cvsroot/mozilla/toolkit/xre/Makefile.in,v  <--  Makefile.in
new revision: 1.93; previous revision: 1.92
done
Checking in toolkit/xre/nsAppRunner.cpp;
/cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.214; previous revision: 1.213
done
Checking in xpcom/base/nsObjCExceptions.h;
/cvsroot/mozilla/xpcom/base/nsObjCExceptions.h,v  <--  nsObjCExceptions.h
new revision: 1.7; previous revision: 1.6
done
Checking in xpcom/system/nsICrashReporter.idl;
/cvsroot/mozilla/xpcom/system/nsICrashReporter.idl,v  <--  nsICrashReporter.idl
new revision: 1.2; previous revision: 1.1
done

Comment 13

[(not currently active) Ted Mielczarek]

This fix can be verified along with bug 451709 in the steps in bug 451709 comment 18.

Comment 14

[Bob Clary [:bc] (inactive)]

I believe this caused Bug 465233

Comment 15

[Al Billings [:abillings - ex-MoCo]]

I verified this for 1.9.0.5 when I verified bug 451709.