Fixes #691 (#696)

* Fixes #691 Sometimes native crashes are swallwed by the Crash reporter. Hence the uncaught exception handler prints them, and the Crash reporter also returns the correct fingerprint, so the crash can be discovered. * Fix minor review comment
tessel · Apr 22, 2016 · 38a77f7 · 38a77f7
1 parent d78991a
commit 38a77f7
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 14 deletions.
diff --git a/lib/crash-reporter.js b/lib/crash-reporter.js
@@ -13,13 +13,14 @@ var Preferences = require('./preferences');
 // the value of the crash reporter preference
 // the value has to be one of 'on' or 'off'
 var CRASH_REPORTER_PREFERENCE = 'crash.reporter.preference';
-var SUBMIT_CRASH_URL = 'http://crash-reporter.tessel.io/crashes/submit';
+var CRASH_REPORTER_BASE_URL = 'http://crash-reporter.tessel.io';
 
 // override for testing
 if (process.env.DEV_MODE === 'true') {
-  SUBMIT_CRASH_URL = 'http://localhost:8080/crashes/submit';
+  CRASH_REPORTER_BASE_URL = 'http://localhost:8080';
 }
 
+var SUBMIT_CRASH_URL = `${CRASH_REPORTER_BASE_URL}/crashes/submit`;
 var rPathEscape = /[|\\{}()[\]^$+*?.]/g;
 
 function escape(p) {
@@ -65,16 +66,12 @@ CrashReporter.submit = function(report) {
           stack = stack.replace(new RegExp(escape(__dirname.slice(0, index)), 'g'), '');
         }
 
-        CrashReporter.post(labels, stack)
-          .then(() => {
-            logs.info('Crash Reported: http://crash-reporter.tessel.io/');
-          })
-          .catch(error => {
-            logs.err('Error submitting crash report', error);
+        return CrashReporter.post(labels, stack)
+          .then(fingerprint => {
+            logs.info(`Crash Reported: ${CRASH_REPORTER_BASE_URL}/crashes?fingerprint=${fingerprint}`);
           });
       }
-    })
-    .catch(error => {
+    }).catch(error => {
       // do nothing
       // do not crash the crash reporter :)
       logs.err('Error submitting crash report', error);
@@ -99,7 +96,7 @@ CrashReporter.post = function(labels, report) {
           if (json.error) {
             reject(json.error);
           } else {
-            resolve();
+            resolve(json.crash_report.fingerprint);
           }
         }
       } catch (exception) {
@@ -114,6 +111,9 @@ CrashReporter.test = () => {
 };
 
 var onError = error => {
+  // log the error as sometimes we might be swallowing
+  // unhandled remote node exceptions
+  logs.err('Detected CLI crash', error, error.stack);
   return CrashReporter.submit(error.stack);
 };
 

diff --git a/test/unit/crash-reporter.js b/test/unit/crash-reporter.js
@@ -145,8 +145,14 @@ exports['CrashReporter.post'] = {
     this.pRead = this.sandbox.stub(Preferences, 'read').returns(Promise.resolve('on'));
     this.pWrite = this.sandbox.stub(Preferences, 'write').returns(Promise.resolve());
     this.crPost = this.sandbox.spy(CrashReporter, 'post');
+    this.crFingerPrint = '0xabcd';
+    var crPostResponse = {
+      crash_report: {
+        fingerprint: this.crFingerPrint
+      }
+    };
     this.request = this.sandbox.stub(request, 'post', (opts, handler) => {
-      return handler(null, {}, '{}');
+      return handler(null, {}, JSON.stringify(crPostResponse));
     });
     done();
   },
@@ -157,17 +163,18 @@ exports['CrashReporter.post'] = {
   },
 
   post: function(test) {
-    test.expect(4);
+    test.expect(5);
 
     var labels = 'foo';
     var report = 'Error: undefined is not a function';
 
-    CrashReporter.post(labels, report).then(() => {
+    CrashReporter.post(labels, report).then(fingerprint => {
       var args = this.request.lastCall.args;
       test.equal(this.request.callCount, 1);
       test.equal(args[0].form.crash, report);
       test.equal(args[0].form.labels, labels);
       test.equal(args[0].form.f, 'json');
+      test.equal(fingerprint, this.crFingerPrint);
       test.done();
     });
   },