From 6cb5c63b15b51f3b09423771b3be03d56d33a5cd Mon Sep 17 00:00:00 2001 From: "dcamp@mozilla.com" Date: Sat, 23 Jun 2007 22:39:51 -0700 Subject: [PATCH] Improve crash reporter errors. b=385359, r=luser --- toolkit/airbag/client/crashreporter.cpp | 114 +++++++++++++----- toolkit/airbag/client/crashreporter.h | 41 +++++-- toolkit/airbag/client/crashreporter.ini | 17 ++- toolkit/airbag/client/crashreporter_linux.cpp | 12 +- toolkit/airbag/client/crashreporter_osx.mm | 15 ++- toolkit/airbag/client/crashreporter_win.cpp | 8 +- 6 files changed, 159 insertions(+), 48 deletions(-) diff --git a/toolkit/airbag/client/crashreporter.cpp b/toolkit/airbag/client/crashreporter.cpp index d143ac0a5dea..82b5b30443ff 100644 --- a/toolkit/airbag/client/crashreporter.cpp +++ b/toolkit/airbag/client/crashreporter.cpp @@ -68,6 +68,22 @@ static string gExtraFile; static string kExtraDataExtension = ".extra"; +void UIError(const string& message) +{ + string errorMessage; + if (!gStrings[ST_CRASHREPORTERERROR].empty()) { + char buf[2048]; + UI_SNPRINTF(buf, 2048, + gStrings[ST_CRASHREPORTERERROR].c_str(), + message.c_str()); + errorMessage = buf; + } else { + errorMessage = message; + } + + UIError_impl(errorMessage); +} + static string Unescape(const string& str) { string ret; @@ -210,14 +226,20 @@ static bool MoveCrashData(const string& toDir, string& extrafile) { if (!UIEnsurePathExists(toDir)) { + UIError(gStrings[ST_ERROR_CREATEDUMPDIR]); return false; } string newDump = toDir + UI_DIR_SEPARATOR + Basename(dumpfile); string newExtra = toDir + UI_DIR_SEPARATOR + Basename(extrafile); - if (!UIMoveFile(dumpfile, newDump) || - !UIMoveFile(extrafile, newExtra)) { + if (!UIMoveFile(dumpfile, newDump)) { + UIError(gStrings[ST_ERROR_DUMPFILEMOVE]); + return false; + } + + if (!UIMoveFile(extrafile, newExtra)) { + UIError(gStrings[ST_ERROR_EXTRAFILEMOVE]); return false; } @@ -288,13 +310,52 @@ bool SendCompleted(bool success, const string& serverResponse) using namespace CrashReporter; +void RewriteStrings(StringTable& queryParameters) +{ + // rewrite some UI strings with the values from the query parameters + string product = queryParameters["ProductName"]; + string vendor = queryParameters["Vendor"]; + if (vendor.empty()) { + // Assume Mozilla if no vendor is specified + vendor = "Mozilla"; + } + + char buf[4096]; + UI_SNPRINTF(buf, sizeof(buf), + gStrings[ST_CRASHREPORTERVENDORTITLE].c_str(), + vendor.c_str()); + gStrings[ST_CRASHREPORTERTITLE] = buf; + + // Leave a format specifier for UIError to fill in + UI_SNPRINTF(buf, sizeof(buf), + gStrings[ST_CRASHREPORTERPRODUCTERROR].c_str(), + product.c_str(), + "%s"); + gStrings[ST_CRASHREPORTERERROR] = buf; + + UI_SNPRINTF(buf, sizeof(buf), + gStrings[ST_CRASHREPORTERDESCRIPTION].c_str(), + product.c_str()); + gStrings[ST_CRASHREPORTERDESCRIPTION] = buf; + + UI_SNPRINTF(buf, sizeof(buf), + gStrings[ST_CHECKSUBMIT].c_str(), + vendor.c_str()); + gStrings[ST_CHECKSUBMIT] = buf; + + UI_SNPRINTF(buf, sizeof(buf), + gStrings[ST_RESTART].c_str(), + product.c_str()); + gStrings[ST_RESTART] = buf; +} + int main(int argc, char** argv) { gArgc = argc; gArgv = argv; if (!ReadConfig()) { - UIError("Couldn't read configuration"); + UIError("Couldn't read configuration."); return 0; } @@ -311,36 +372,49 @@ int main(int argc, char** argv) } else { gExtraFile = GetExtraDataFilename(gDumpFile); if (gExtraFile.empty()) { - UIError("Couldn't get extra data filename"); + UIError(gStrings[ST_ERROR_BADARGUMENTS]); + return 0; + } + + if (!UIFileExists(gExtraFile)) { + UIError(gStrings[ST_ERROR_EXTRAFILEEXISTS]); return 0; } StringTable queryParameters; if (!ReadStringsFromFile(gExtraFile, queryParameters, true)) { - UIError("Couldn't read extra data"); + UIError(gStrings[ST_ERROR_EXTRAFILEREAD]); return 0; } if (queryParameters.find("ProductName") == queryParameters.end()) { - UIError("No product name specified"); + UIError(gStrings[ST_ERROR_NOPRODUCTNAME]); return 0; } + // There is enough information in the extra file to rewrite strings + // to be product specific + RewriteStrings(queryParameters); + if (queryParameters.find("ServerURL") == queryParameters.end()) { - UIError("No server URL specified"); + UIError(gStrings[ST_ERROR_NOSERVERURL]); return 0; } string product = queryParameters["ProductName"]; string vendor = queryParameters["Vendor"]; if (!UIGetSettingsPath(vendor, product, gSettingsPath)) { - UIError("Couldn't get settings path"); + UIError(gStrings[ST_ERROR_NOSETTINGSPATH]); + return 0; + } + + if (!UIFileExists(gDumpFile)) { + UIError(gStrings[ST_ERROR_DUMPFILEEXISTS]); return 0; } string pendingDir = gSettingsPath + UI_DIR_SEPARATOR + "pending"; if (!MoveCrashData(pendingDir, gDumpFile, gExtraFile)) { - UIError("Couldn't move crash data"); return 0; } @@ -370,28 +444,6 @@ int main(int argc, char** argv) sendURL = urlEnv; } - // rewrite some UI strings with the values from the query parameters - char buf[4096]; - UI_SNPRINTF(buf, sizeof(buf), - gStrings[ST_RESTART].c_str(), - product.c_str()); - gStrings[ST_RESTART] = buf; - - UI_SNPRINTF(buf, sizeof(buf), - gStrings[ST_CRASHREPORTERDESCRIPTION].c_str(), - product.c_str()); - gStrings[ST_CRASHREPORTERDESCRIPTION] = buf; - - UI_SNPRINTF(buf, sizeof(buf), - gStrings[ST_CHECKSUBMIT].c_str(), - vendor.empty() ? "Mozilla" : vendor.c_str()); - gStrings[ST_CHECKSUBMIT] = buf; - - UI_SNPRINTF(buf, sizeof(buf), - gStrings[ST_CRASHREPORTERTITLE].c_str(), - vendor.empty() ? "Mozilla" : vendor.c_str()); - gStrings[ST_CRASHREPORTERTITLE] = buf; - UIShowCrashUI(gDumpFile, queryParameters, sendURL, restartArgs); } diff --git a/toolkit/airbag/client/crashreporter.h b/toolkit/airbag/client/crashreporter.h index e0ba9ef05a20..ec7fe469e553 100644 --- a/toolkit/airbag/client/crashreporter.h +++ b/toolkit/airbag/client/crashreporter.h @@ -29,17 +29,31 @@ typedef std::map StringTable; -#define ST_CRASHREPORTERTITLE "CrashReporterTitle" -#define ST_CRASHREPORTERHEADER "CrashReporterHeader" -#define ST_CRASHREPORTERDESCRIPTION "CrashReporterDescription" -#define ST_CRASHREPORTERDEFAULT "CrashReporterDefault" -#define ST_VIEWREPORT "ViewReport" -#define ST_EXTRAREPORTINFO "ExtraReportInfo" -#define ST_CHECKSUBMIT "CheckSubmit" -#define ST_CHECKEMAIL "CheckEmail" -#define ST_CLOSE "Close" -#define ST_RESTART "Restart" -#define ST_SUBMITFAILED "SubmitFailed" +#define ST_CRASHREPORTERTITLE "CrashReporterTitle" +#define ST_CRASHREPORTERVENDORTITLE "CrashReporterVendorTitle" +#define ST_CRASHREPORTERERROR "CrashReporterError" +#define ST_CRASHREPORTERPRODUCTERROR "CrashReporterProductError" +#define ST_CRASHREPORTERHEADER "CrashReporterHeader" +#define ST_CRASHREPORTERDESCRIPTION "CrashReporterDescription" +#define ST_CRASHREPORTERDEFAULT "CrashReporterDefault" +#define ST_VIEWREPORT "ViewReport" +#define ST_EXTRAREPORTINFO "ExtraReportInfo" +#define ST_CHECKSUBMIT "CheckSubmit" +#define ST_CHECKEMAIL "CheckEmail" +#define ST_CLOSE "Close" +#define ST_RESTART "Restart" +#define ST_SUBMITFAILED "SubmitFailed" + +#define ST_ERROR_BADARGUMENTS "ErrorBadArguments" +#define ST_ERROR_EXTRAFILEEXISTS "ErrorExtraFileExists" +#define ST_ERROR_EXTRAFILEREAD "ErrorExtraFileRead" +#define ST_ERROR_EXTRAFILEMOVE "ErrorExtraFileMove" +#define ST_ERROR_DUMPFILEEXISTS "ErrorDumpFileExists" +#define ST_ERROR_DUMPFILEMOVE "ErrorDumpFileMove" +#define ST_ERROR_NOPRODUCTNAME "ErrorNoProductName" +#define ST_ERROR_NOSERVERURL "ErrorNoServerURL" +#define ST_ERROR_NOSETTINGSPATH "ErrorNoSettingsPath" +#define ST_ERROR_CREATEDUMPDIR "ErrorCreateDumpDir" //============================================================================= // implemented in crashreporter.cpp @@ -51,6 +65,8 @@ namespace CrashReporter { extern int gArgc; extern char** gArgv; + void UIError(const std::string& message); + // The UI finished sending the report bool SendCompleted(bool success, const std::string& serverResponse); @@ -87,13 +103,14 @@ void UIShowCrashUI(const std::string& dumpfile, const std::string& sendURL, const std::vector& restartArgs); -void UIError(const std::string& message); +void UIError_impl(const std::string& message); bool UIGetIniPath(std::string& path); bool UIGetSettingsPath(const std::string& vendor, const std::string& product, std::string& settingsPath); bool UIEnsurePathExists(const std::string& path); +bool UIFileExists(const std::string& path); bool UIMoveFile(const std::string& oldfile, const std::string& newfile); bool UIDeleteFile(const std::string& oldfile); diff --git a/toolkit/airbag/client/crashreporter.ini b/toolkit/airbag/client/crashreporter.ini index 1176d4b310ce..48b24307323c 100644 --- a/toolkit/airbag/client/crashreporter.ini +++ b/toolkit/airbag/client/crashreporter.ini @@ -1,6 +1,9 @@ ; This file is in the UTF-8 encoding [Strings] -CrashReporterTitle=%s Crash Reporter +CrashReporterTitle=Crash Reporter +CrashReporterVendorTitle=%s Crash Reporter +CrashReporterError=We're sorry, but the application hit an unexpected problem and crashed.\n\nUnfortunately the crash reporter is unable to submit a report for this crash.\n\nDetails: %s +CrashReporterProductError=We're sorry, but %s hit an unexpected problem and crashed. We'll try to restore your tabs and windows when it restarts.\n\nUnfortunately the crash reporter is unable to submit a crash report.\n\nDetails: %s CrashReporterHeader=Crash! Bang! Boom! CrashReporterDescription=We're sorry, but %s hit an unexpected problem and crashed. We'll try to restore your tabs and windows when it restarts.\n\nTo help us diagnose and repair this problem, you can send us a crash report. CrashReporterDefault=This application is run after a crash to report the problem to the application vendor. It should not be run directly. @@ -13,3 +16,15 @@ Restart=Restart %s SubmitFailed=Failed to submit crash report CrashID=Crash ID: %s CrashDetailsURL=You can view details of this crash at %s + +ErrorBadArguments=The application passed an invalid argument. +ErrorExtraFileExists=The application didn't leave an application data file. +ErrorExtraFileRead=Couldn't read the application data file. +ErrorExtraFileMove=Couldn't move application data file. +ErrorDumpFileExists=The application did not leave a crash dump file. +ErrorDumpFileMove=Couldn't move crash dump. +ErrorNoProductName=The application did not identify itself. +ErrorNoServerURL=The application did not specify a crash reporting server. +ErrorNoSettingsPath=Couldn't find the crash reporter's settings. +ErrorCreateDumpDir=Couldn't create pending dump directory. + diff --git a/toolkit/airbag/client/crashreporter_linux.cpp b/toolkit/airbag/client/crashreporter_linux.cpp index 1bacc74c9904..d47151006cf1 100644 --- a/toolkit/airbag/client/crashreporter_linux.cpp +++ b/toolkit/airbag/client/crashreporter_linux.cpp @@ -412,7 +412,7 @@ void UIShowCrashUI(const string& dumpfile, gtk_main(); } -void UIError(const string& message) +void UIError_impl(const string& message) { if (!gInitialized) { // Didn't initialize, this is the best we can do @@ -477,6 +477,16 @@ bool UIEnsurePathExists(const string& path) return true; } +bool UIFileExists(const string& path) +{ + struct stat sb; + int ret = stat(path.c_str(), &sb); + if (ret == -1 || !(sb.st_mode & S_IFREG)) + return false; + + return true; +} + bool UIMoveFile(const string& file, const string& newfile) { return (rename(file.c_str(), newfile.c_str()) != -1); diff --git a/toolkit/airbag/client/crashreporter_osx.mm b/toolkit/airbag/client/crashreporter_osx.mm index f2ce929a8afb..1cbb2a2a3986 100644 --- a/toolkit/airbag/client/crashreporter_osx.mm +++ b/toolkit/airbag/client/crashreporter_osx.mm @@ -428,7 +428,8 @@ void UIShutdown() void UIShowDefaultUI() { - UIError(gStrings[ST_CRASHREPORTERDEFAULT]); + [gUI showErrorUI: gStrings[ST_CRASHREPORTERDEFAULT]]; + [NSApp run]; } void UIShowCrashUI(const string& dumpfile, @@ -444,7 +445,7 @@ void UIShowCrashUI(const string& dumpfile, [NSApp run]; } -void UIError(const string& message) +void UIError_impl(const string& message) { if (!gUI) { // UI failed to initialize, printing is the best we can do @@ -502,6 +503,16 @@ bool UIEnsurePathExists(const string& path) return true; } +bool UIFileExists(const string& path) +{ + struct stat sb; + int ret = stat(path.c_str(), &sb); + if (ret == -1 || !(sb.st_mode & S_IFREG)) + return false; + + return true; +} + bool UIMoveFile(const string& file, const string& newfile) { return (rename(file.c_str(), newfile.c_str()) != -1); diff --git a/toolkit/airbag/client/crashreporter_win.cpp b/toolkit/airbag/client/crashreporter_win.cpp index 8713905fbbb0..14f7aaf85a14 100644 --- a/toolkit/airbag/client/crashreporter_win.cpp +++ b/toolkit/airbag/client/crashreporter_win.cpp @@ -706,7 +706,7 @@ void UIShowCrashUI(const string& dumpFile, (DLGPROC)CrashReporterDialogProc, 0); } -void UIError(const string& message) +void UIError_impl(const string& message) { wstring title = Str(ST_CRASHREPORTERTITLE); if (title.empty()) @@ -765,6 +765,12 @@ bool UIEnsurePathExists(const string& path) return true; } +bool UIFileExists(const string& path) +{ + DWORD attrs = GetFileAttributes(UTF8ToWide(path).c_str()); + return (attrs != INVALID_FILE_ATTRIBUTES); +} + bool UIMoveFile(const string& oldfile, const string& newfile) { if (oldfile == newfile)