Merge pull request #810 from atom/report-back-crash-id

WIP: Add API to get last crash report ID

atom/common/api/lib/crash-reporter.coffee

@@ -1,18 +1,21 @@
-{spawn} = require 'child_process'
 binding = process.atomBinding 'crash_reporter'
+fs      = require 'fs'
+os      = require 'os'
+path    = require 'path'
+{spawn} = require 'child_process'
 
 class CrashReporter
   start: (options={}) ->
-    {productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra} = options
+    {@productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra} = options
 
-    productName ?= 'Atom-Shell'
+    @productName ?= 'Atom-Shell'
     companyName ?= 'GitHub, Inc'
     submitUrl ?= 'http://54.249.141.255:1127/post'
     autoSubmit ?= true
     ignoreSystemCrashHandler ?= false
     extra ?= {}
 
-    extra._productName ?= productName
+    extra._productName ?= @productName
     extra._companyName ?= companyName
     extra._version ?=
       if process.type is 'browser'
@@ -20,12 +23,12 @@ class CrashReporter
       else
         require('remote').require('app').getVersion()
 
-    start = -> binding.start productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra
+    start = => binding.start @productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra
 
     if process.platform is 'win32'
       args = [
         "--reporter-url=#{submitUrl}"
-        "--application-name=#{productName}"
+        "--application-name=#{@productName}"
         "--v=1"
       ]
       env = ATOM_SHELL_INTERNAL_CRASH_SERVICE: 1
@@ -35,4 +38,20 @@ class CrashReporter
     else
       start()
 
-module.exports = new CrashReporter
+  getLastCrashReport: ->
+    tmpdir =
+      if process.platform is 'win32'
+        os.tmpdir()
+      else
+        '/tmp'
+    log = path.join tmpdir, "#{@productName} Crashes", 'uploads.log'
+    try
+      reports = String(fs.readFileSync(log)).split('\n')
+      return null unless reports.length > 1
+      [time, id] = reports[reports.length - 2].split ','
+      return {date: new Date(parseInt(time) * 1000), id}
+    catch e
+      return null
+
+crashRepoter = new CrashReporter
+module.exports = crashRepoter

atom/common/crash_reporter/crash_reporter_linux.cc

@@ -12,11 +12,12 @@
 
 #include "base/debug/crash_logging.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/linux_util.h"
 #include "base/logging.h"
-#include "base/path_service.h"
 #include "base/process/memory.h"
 #include "base/memory/singleton.h"
+#include "base/strings/stringprintf.h"
 #include "vendor/breakpad/src/client/linux/handler/exception_handler.h"
 #include "vendor/breakpad/src/common/linux/linux_libc_support.h"
 
@@ -61,7 +62,7 @@ void CrashReporterLinux::InitBreakpad(const std::string& product_name,
                                       const std::string& submit_url,
                                       bool auto_submit,
                                       bool skip_system_crash_handler) {
-  EnableCrashDumping();
+  EnableCrashDumping(product_name);
 
   crash_keys_.SetKeyValue("prod", "Atom-Shell");
   crash_keys_.SetKeyValue("ver", version.c_str());
@@ -76,11 +77,15 @@ void CrashReporterLinux::SetUploadParameters() {
   upload_parameters_["platform"] = "linux";
 }
 
-void CrashReporterLinux::EnableCrashDumping() {
+void CrashReporterLinux::EnableCrashDumping(const std::string& product_name) {
-  base::FilePath tmp_path("/tmp");
+  std::string dump_dir = "/tmp/" + product_name + " Crashes";
-  PathService::Get(base::DIR_TEMP, &tmp_path);
+  base::FilePath dumps_path(dump_dir);
+  base::CreateDirectory(dumps_path);
+
+  std::string log_file = base::StringPrintf(
+      "%s/%s", dump_dir.c_str(), "uploads.log");
+  strncpy(g_crash_log_path, log_file.c_str(), sizeof(g_crash_log_path));
 
-  base::FilePath dumps_path(tmp_path);
   MinidumpDescriptor minidump_descriptor(dumps_path.value());
   minidump_descriptor.set_size_limit(kMaxMinidumpFileSize);
 

atom/common/crash_reporter/crash_reporter_linux.h

@@ -39,7 +39,7 @@ class CrashReporterLinux : public CrashReporter {
   CrashReporterLinux();
   virtual ~CrashReporterLinux();
 
-  void EnableCrashDumping();
+  void EnableCrashDumping(const std::string& product_name);
 
   static bool CrashDone(const google_breakpad::MinidumpDescriptor& minidump,
                         void* context,

atom/common/crash_reporter/linux/crash_dump_handler.cc

@@ -75,6 +75,10 @@ uint64_t kernel_timeval_to_ms(struct kernel_timeval *tv) {
   return ret;
 }
 
+bool my_isxdigit(char c) {
+  return (c >= '0' && c <= '9') || ((c | 0x20) >= 'a' && (c | 0x20) <= 'f');
+}
+
 size_t LengthWithoutTrailingSpaces(const char* str, size_t len) {
   while (len > 0 && str[len - 1] == ' ') {
     len--;
@@ -345,13 +349,12 @@ void ExecUploadProcessOrTerminate(const BreakpadInfo& info,
     kWgetBinary,
     header,
     post_file,
-    // TODO(zcbenz): Enabling custom upload url.
     info.upload_url,
     "--timeout=60",  // Set a timeout so we don't hang forever.
     "--tries=1",     // Don't retry if the upload fails.
     "--quiet",       // Be silent.
     "-O",            // output reply to /dev/null.
-    "/dev/null",
+    "/dev/fd/3",
     NULL,
   };
   static const char msg[] = "Cannot upload crash dump: cannot exec "
@@ -361,8 +364,98 @@ void ExecUploadProcessOrTerminate(const BreakpadInfo& info,
   sys__exit(1);
 }
 
+// Runs in the helper process to wait for the upload process running
+// ExecUploadProcessOrTerminate() to finish. Returns the number of bytes written
+// to |fd| and save the written contents to |buf|.
+// |buf| needs to be big enough to hold |bytes_to_read| + 1 characters.
+size_t WaitForCrashReportUploadProcess(int fd, size_t bytes_to_read,
+                                       char* buf) {
+  size_t bytes_read = 0;
+
+  // Upload should finish in about 10 seconds. Add a few more 500 ms
+  // internals to account for process startup time.
+  for (size_t wait_count = 0; wait_count < 24; ++wait_count) {
+    struct kernel_pollfd poll_fd;
+    poll_fd.fd = fd;
+    poll_fd.events = POLLIN | POLLPRI | POLLERR;
+    int ret = sys_poll(&poll_fd, 1, 500);
+    if (ret < 0) {
+      // Error
+      break;
+    } else if (ret > 0) {
+      // There is data to read.
+      ssize_t len = HANDLE_EINTR(
+          sys_read(fd, buf + bytes_read, bytes_to_read - bytes_read));
+      if (len < 0)
+        break;
+      bytes_read += len;
+      if (bytes_read == bytes_to_read)
+        break;
+    }
+    // |ret| == 0 -> timed out, continue waiting.
+    // or |bytes_read| < |bytes_to_read| still, keep reading.
+  }
+  buf[bytes_to_read] = 0;  // Always NUL terminate the buffer.
+  return bytes_read;
+}
+
+// |buf| should be |expected_len| + 1 characters in size and NULL terminated.
+bool IsValidCrashReportId(const char* buf, size_t bytes_read,
+                          size_t expected_len) {
+  if (bytes_read != expected_len)
+    return false;
+  for (size_t i = 0; i < bytes_read; ++i) {
+    if (!my_isxdigit(buf[i]) && buf[i] != '-')
+      return false;
+  }
+  return true;
+}
+
+// |buf| should be |expected_len| + 1 characters in size and NULL terminated.
+void HandleCrashReportId(const char* buf, size_t bytes_read,
+                         size_t expected_len) {
+  if (!IsValidCrashReportId(buf, bytes_read, expected_len)) {
+    static const char msg[] = "Failed to get crash dump id.";
+    WriteLog(msg, sizeof(msg) - 1);
+    WriteNewline();
+
+    static const char id_msg[] = "Report Id: ";
+    WriteLog(id_msg, sizeof(id_msg) - 1);
+    WriteLog(buf, bytes_read);
+    WriteNewline();
+    return;
+  }
+
+  // Write crash dump id to stderr.
+  static const char msg[] = "Crash dump id: ";
+  WriteLog(msg, sizeof(msg) - 1);
+  WriteLog(buf, my_strlen(buf));
+  WriteNewline();
+
+  // Write crash dump id to crash log as: seconds_since_epoch,crash_id
+  struct kernel_timeval tv;
+  if (!sys_gettimeofday(&tv, NULL)) {
+    uint64_t time = kernel_timeval_to_ms(&tv) / 1000;
+    char time_str[kUint64StringSize];
+    const unsigned time_len = my_uint64_len(time);
+    my_uint64tos(time_str, time, time_len);
+
+    const int kLogOpenFlags = O_CREAT | O_WRONLY | O_APPEND | O_CLOEXEC;
+    int log_fd = sys_open(g_crash_log_path, kLogOpenFlags, 0600);
+    if (log_fd > 0) {
+      sys_write(log_fd, time_str, time_len);
+      sys_write(log_fd, ",", 1);
+      sys_write(log_fd, buf, my_strlen(buf));
+      sys_write(log_fd, "\n", 1);
+      IGNORE_RET(sys_close(log_fd));
+    }
+  }
+}
+
 }  // namespace
 
+char g_crash_log_path[256];
+
 void HandleCrashDump(const BreakpadInfo& info) {
   int dumpfd;
   bool keep_fd = false;
@@ -616,33 +709,13 @@ void HandleCrashDump(const BreakpadInfo& info) {
       // Helper process.
       if (upload_child > 0) {
         IGNORE_RET(sys_close(fds[1]));
-        char id_buf[17];  // Crash report IDs are expected to be 16 chars.
+
-        ssize_t len = -1;
+        const size_t kCrashIdLength = 36;
-        // Upload should finish in about 10 seconds. Add a few more 500 ms
+        char id_buf[kCrashIdLength + 1];
-        // internals to account for process startup time.
+        size_t bytes_read =
-        for (size_t wait_count = 0; wait_count < 24; ++wait_count) {
+            WaitForCrashReportUploadProcess(fds[0], kCrashIdLength, id_buf);
-          struct kernel_pollfd poll_fd;
+        HandleCrashReportId(id_buf, bytes_read, kCrashIdLength);
-          poll_fd.fd = fds[0];
+
-          poll_fd.events = POLLIN | POLLPRI | POLLERR;
-          int ret = sys_poll(&poll_fd, 1, 500);
-          if (ret < 0) {
-            // Error
-            break;
-          } else if (ret > 0) {
-            // There is data to read.
-            len = HANDLE_EINTR(sys_read(fds[0], id_buf, sizeof(id_buf) - 1));
-            break;
-          }
-          // ret == 0 -> timed out, continue waiting.
-        }
-        if (len > 0) {
-          // Write crash dump id to stderr.
-          id_buf[len] = 0;
-          static const char msg[] = "\nCrash dump id: ";
-          WriteLog(msg, sizeof(msg) - 1);
-          WriteLog(id_buf, my_strlen(id_buf));
-          WriteLog("\n", 1);
-        }
         if (sys_waitpid(upload_child, NULL, WNOHANG) == 0) {
           // Upload process is still around, kill it.
           sys_kill(upload_child, SIGKILL);
@@ -666,4 +739,8 @@ size_t WriteLog(const char* buf, size_t nbytes) {
   return sys_write(2, buf, nbytes);
 }
 
+size_t WriteNewline() {
+  return WriteLog("\n", 1);
+}
+
 }  // namespace crash_reporter

atom/common/crash_reporter/linux/crash_dump_handler.h

@@ -32,6 +32,10 @@ struct BreakpadInfo {
 void HandleCrashDump(const BreakpadInfo& info);
 
 size_t WriteLog(const char* buf, size_t nbytes);
+size_t WriteNewline();
+
+// Global variable storing the path of upload log.
+extern char g_crash_log_path[256];
 
 }  // namespace crash_reporter
 

atom/common/crash_reporter/win/crash_service.cc

@@ -13,6 +13,8 @@
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/time/time.h"
 #include "base/win/windows_version.h"
 #include "vendor/breakpad/src/client/windows/crash_generation/client_info.h"
 #include "vendor/breakpad/src/client/windows/crash_generation/crash_generation_server.h"
@@ -66,6 +68,30 @@ bool WriteCustomInfoToFile(const std::wstring& dump_path, const CrashMap& map) {
   return true;
 }
 
+bool WriteReportIDToFile(const std::wstring& dump_path,
+                         const std::wstring& report_id) {
+  std::wstring file_path(dump_path);
+  size_t last_slash = file_path.rfind(L'\\');
+  if (last_slash == std::wstring::npos)
+    return false;
+  file_path.resize(last_slash);
+  file_path += L"\\uploads.log";
+
+  std::wofstream file(file_path.c_str(),
+      std::ios_base::out | std::ios_base::app | std::ios::binary);
+  if (!file.is_open())
+    return false;
+
+  int64 seconds_since_epoch =
+      (base::Time::Now() - base::Time::UnixEpoch()).InSeconds();
+  std::wstring line = base::Int64ToString16(seconds_since_epoch);
+  line += L',';
+  line += report_id;
+  line += L'\n';
+  file.write(line.c_str(), static_cast<std::streamsize>(line.length()));
+  return true;
+}
+
 // The window procedure task is to handle when a) the user logs off.
 // b) the system shuts down or c) when the user closes the window.
 LRESULT __stdcall CrashSvcWndProc(HWND hwnd, UINT message,
@@ -422,6 +448,7 @@ DWORD CrashService::AsyncSendDump(void* context) {
           ++info->self->requests_sent_;
           ++info->self->requests_handled_;
           retry_round = 0;
+          WriteReportIDToFile(info->dump_path, report_id);
           break;
         case google_breakpad::RESULT_THROTTLED:
           report_id = L"<throttled>";

docs/api/crash-reporter.md

@@ -27,6 +27,11 @@ crashReporter.start({
     * Only string properties are send correctly.
     * Nested objects are not supported.
 
+## crashReporter.getLastCrashReport()
+
+Returns the date and ID of last crash report, when there was no crash report
+sent or the crash reporter is not started, `null` will be returned.
+
 # crash-reporter payload
 
 The crash reporter will send the following data to the `submitUrl` as `POST`:

spec/api-crash-reporter-spec.coffee

@@ -32,7 +32,7 @@ describe 'crash-reporter module', ->
         assert.equal fields['_version'], require('remote').require('app').getVersion()
         assert files['upload_file_minidump']['name']?
 
-        res.end()
+        res.end('abc-123-def')
         server.close()
         done()
     server.listen 0, '127.0.0.1', ->