Merge pull request #8109 from ramya-rao-a/crash-for-children

Expose crash reporter start for child node processes

atom/app/node_main.cc

@@ -7,18 +7,16 @@
 #include "atom/app/uv_task_runner.h"
 #include "atom/browser/javascript_environment.h"
 #include "atom/browser/node_debugger.h"
+#include "atom/common/api/atom_bindings.h"
+#include "atom/common/crash_reporter/crash_reporter.h"
+#include "atom/common/native_mate_converters/string16_converter.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "gin/array_buffer.h"
 #include "gin/public/isolate_holder.h"
 #include "gin/v8_initializer.h"
-
-#if defined(OS_WIN)
-#include "atom/common/api/atom_bindings.h"
-#include "atom/common/native_mate_converters/string16_converter.h"
 #include "native_mate/dictionary.h"
-#endif
 
 #include "atom/common/node_includes.h"
 
@@ -58,10 +56,16 @@ int NodeMain(int argc, char *argv[]) {
     if (node_debugger.IsRunning())
       env->AssignToContext(v8::Debug::GetDebugContext(gin_env.isolate()));
 
-#if defined(OS_WIN)
     mate::Dictionary process(gin_env.isolate(), env->process_object());
+#if defined(OS_WIN)
     process.SetMethod("log", &AtomBindings::Log);
 #endif
+    process.SetMethod("crash", &AtomBindings::Crash);
+
+    // Setup process.crashReporter.start in child node processes
+    auto reporter = mate::Dictionary::CreateEmpty(gin_env.isolate());
+    reporter.SetMethod("start", &crash_reporter::CrashReporter::StartInstance);
+    process.Set("crashReporter", reporter);
 
     node::LoadEnvironment(env);
 

atom/common/api/atom_bindings.cc

@@ -23,10 +23,6 @@ namespace {
 // Dummy class type that used for crashing the program.
 struct DummyClass { bool crash; };
 
-void Crash() {
-  static_cast<DummyClass*>(nullptr)->crash = true;
-}
-
 void Hang() {
   for (;;)
     base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(1));
@@ -76,7 +72,7 @@ v8::Local<v8::Value> GetSystemMemoryInfo(v8::Isolate* isolate,
 // we can get the stack trace.
 void FatalErrorCallback(const char* location, const char* message) {
   LOG(ERROR) << "Fatal error in V8: " << location << " " << message;
-  Crash();
+  AtomBindings::Crash();
 }
 
 }  // namespace
@@ -95,7 +91,7 @@ void AtomBindings::BindTo(v8::Isolate* isolate,
   v8::V8::SetFatalErrorHandler(FatalErrorCallback);
 
   mate::Dictionary dict(isolate, process);
-  dict.SetMethod("crash", &Crash);
+  dict.SetMethod("crash", &AtomBindings::Crash);
   dict.SetMethod("hang", &Hang);
   dict.SetMethod("log", &Log);
   dict.SetMethod("getProcessMemoryInfo", &GetProcessMemoryInfo);
@@ -159,4 +155,9 @@ void AtomBindings::Log(const base::string16& message) {
   std::cout << message << std::flush;
 }
 
+// static
+void AtomBindings::Crash() {
+  static_cast<DummyClass*>(nullptr)->crash = true;
+}
+
 }  // namespace atom

atom/common/api/atom_bindings.h

@@ -28,6 +28,7 @@ class AtomBindings {
   void BindTo(v8::Isolate* isolate, v8::Local<v8::Object> process);
 
   static void Log(const base::string16& message);
+  static void Crash();
 
  private:
   void ActivateUVLoop(v8::Isolate* isolate);

atom/common/crash_reporter/crash_reporter.cc

@@ -6,6 +6,7 @@
 
 #include "atom/browser/browser.h"
 #include "atom/common/atom_version.h"
+#include "atom/common/native_mate_converters/file_path_converter.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -93,4 +94,26 @@ CrashReporter* CrashReporter::GetInstance() {
 }
 #endif
 
+void CrashReporter::StartInstance(const mate::Dictionary& options) {
+  auto reporter = GetInstance();
+  if (!reporter) return;
+
+  std::string product_name;
+  options.Get("productName", &product_name);
+  std::string company_name;
+  options.Get("companyName", &company_name);
+  std::string submit_url;
+  options.Get("submitURL", &submit_url);
+  base::FilePath crashes_dir;
+  options.Get("crashesDirectory", &crashes_dir);
+  StringMap extra_parameters;
+  options.Get("extra", &extra_parameters);
+
+  extra_parameters["_productName"] = product_name;
+  extra_parameters["_companyName"] = company_name;
+
+  reporter->Start(product_name, company_name, submit_url, crashes_dir, true,
+                  false, extra_parameters);
+}
+
 }  // namespace crash_reporter

atom/common/crash_reporter/crash_reporter.h

@@ -12,6 +12,7 @@
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "native_mate/dictionary.h"
 
 namespace crash_reporter {
 
@@ -21,6 +22,7 @@ class CrashReporter {
   typedef std::pair<int, std::string> UploadReportResult;  // upload-date, id
 
   static CrashReporter* GetInstance();
+  static void StartInstance(const mate::Dictionary& options);
 
   void Start(const std::string& product_name,
              const std::string& company_name,

spec/api-crash-reporter-spec.js

@@ -1,4 +1,5 @@
 const assert = require('assert')
+const childProcess = require('child_process')
 const http = require('http')
 const multiparty = require('multiparty')
 const path = require('path')
@@ -40,51 +41,34 @@ describe('crashReporter module', function () {
 
     this.timeout(120000)
 
-    var called = false
-    var server = http.createServer(function (req, res) {
-      server.close()
-      var form = new multiparty.Form()
-      form.parse(req, function (error, fields) {
-        if (error) throw error
-        if (called) return
-        called = true
-        assert.equal(fields['prod'], 'Electron')
-        assert.equal(fields['ver'], process.versions.electron)
-        assert.equal(fields['process_type'], 'renderer')
-        assert.equal(fields['platform'], process.platform)
-        assert.equal(fields['extra1'], 'extra1')
-        assert.equal(fields['extra2'], 'extra2')
-        assert.equal(fields['_productName'], 'Zombies')
-        assert.equal(fields['_companyName'], 'Umbrella Corporation')
-        assert.equal(fields['_version'], app.getVersion())
-
-        const reportId = 'abc-123-def-456-abc-789-abc-123-abcd'
-        res.end(reportId, () => {
-          waitForCrashReport().then(() => {
-            assert.equal(crashReporter.getLastCrashReport().id, reportId)
-            assert.notEqual(crashReporter.getUploadedReports().length, 0)
-            assert.equal(crashReporter.getUploadedReports()[0].id, reportId)
-            done()
-          }, done)
+    startServer({
+      callback (port) {
+        const crashUrl = url.format({
+          protocol: 'file',
+          pathname: path.join(fixtures, 'api', 'crash.html'),
+          search: '?port=' + port
         })
-      })
+        w.loadURL(crashUrl)
+      },
+      processType: 'renderer',
+      done: done
     })
-    var port = remote.process.port
-    server.listen(port, '127.0.0.1', function () {
-      port = server.address().port
-      remote.process.port = port
-      const crashUrl = url.format({
-        protocol: 'file',
-        pathname: path.join(fixtures, 'api', 'crash.html'),
-        search: '?port=' + port
-      })
-      if (process.platform === 'darwin') {
-        crashReporter.start({
-          companyName: 'Umbrella Corporation',
-          submitURL: 'http://127.0.0.1:' + port
-        })
-      }
-      w.loadURL(crashUrl)
+  })
+
+  it('should send minidump when node processes crash', function (done) {
+    if (isCI) return done()
+
+    this.timeout(120000)
+
+    startServer({
+      callback (port) {
+        const crashesDir = path.join(app.getPath('temp'), `${app.getName()} Crashes`)
+        const version = app.getVersion()
+        const crashPath = path.join(fixtures, 'module', 'crash.js')
+        childProcess.fork(crashPath, [port, version, crashesDir], {silent: true})
+      },
+      processType: 'browser',
+      done: done
     })
   })
 
@@ -155,3 +139,47 @@ const waitForCrashReport = () => {
     checkForReport()
   })
 }
+
+const startServer = ({callback, processType, done}) => {
+  var called = false
+  var server = http.createServer((req, res) => {
+    server.close()
+    var form = new multiparty.Form()
+    form.parse(req, (error, fields) => {
+      if (error) throw error
+      if (called) return
+      called = true
+      assert.equal(fields.prod, 'Electron')
+      assert.equal(fields.ver, process.versions.electron)
+      assert.equal(fields.process_type, processType)
+      assert.equal(fields.platform, process.platform)
+      assert.equal(fields.extra1, 'extra1')
+      assert.equal(fields.extra2, 'extra2')
+      assert.equal(fields._productName, 'Zombies')
+      assert.equal(fields._companyName, 'Umbrella Corporation')
+      assert.equal(fields._version, app.getVersion())
+
+      const reportId = 'abc-123-def-456-abc-789-abc-123-abcd'
+      res.end(reportId, () => {
+        waitForCrashReport().then(() => {
+          assert.equal(crashReporter.getLastCrashReport().id, reportId)
+          assert.notEqual(crashReporter.getUploadedReports().length, 0)
+          assert.equal(crashReporter.getUploadedReports()[0].id, reportId)
+          done()
+        }, done)
+      })
+    })
+  })
+  let {port} = remote.process
+  server.listen(port, '127.0.0.1', () => {
+    port = server.address().port
+    remote.process.port = port
+    if (process.platform === 'darwin') {
+      crashReporter.start({
+        companyName: 'Umbrella Corporation',
+        submitURL: 'http://127.0.0.1:' + port
+      })
+    }
+    callback(port)
+  })
+}

spec/fixtures/module/crash.js

@@ -0,0 +1,13 @@
+process.crashReporter.start({
+  productName: 'Zombies',
+  companyName: 'Umbrella Corporation',
+  crashesDirectory: process.argv[4],
+  submitURL: `http://127.0.0.1:${process.argv[2]}`,
+  extra: {
+    extra1: 'extra1',
+    extra2: 'extra2',
+    _version: process.argv[3]
+  }
+})
+
+process.nextTick(() => process.crash())