Browse Source

chore: add V8 crash information to crashReporter (#24864)

Shelley Vohr 4 years ago
parent
commit
1aeba811cc

+ 11 - 0
shell/common/api/electron_api_v8_util.cc

@@ -133,6 +133,16 @@ std::vector<v8::Local<v8::Value>> GetWeaklyTrackedValues(v8::Isolate* isolate) {
   }
   return locals;
 }
+
+// This causes a fatal error by creating a circular extension dependency.
+void TriggerFatalErrorForTesting(v8::Isolate* isolate) {
+  static const char* aDeps[] = {"B"};
+  v8::RegisterExtension(std::make_unique<v8::Extension>("A", "", 1, aDeps));
+  static const char* bDeps[] = {"A"};
+  v8::RegisterExtension(std::make_unique<v8::Extension>("B", "", 1, bDeps));
+  v8::ExtensionConfiguration config(1, bDeps);
+  v8::Context::New(isolate, &config);
+}
 #endif
 
 void Initialize(v8::Local<v8::Object> exports,
@@ -160,6 +170,7 @@ void Initialize(v8::Local<v8::Object> exports,
                  &RequestGarbageCollectionForTesting);
   dict.SetMethod("isSameOrigin", &IsSameOrigin);
 #ifdef DCHECK_IS_ON
+  dict.SetMethod("triggerFatalErrorForTesting", &TriggerFatalErrorForTesting);
   dict.SetMethod("getWeaklyTrackedValues", &GetWeaklyTrackedValues);
   dict.SetMethod("clearWeaklyTrackedValues", &ClearWeaklyTrackedValues);
   dict.SetMethod("weaklyTrackValue", &WeaklyTrackValue);

+ 14 - 3
shell/common/api/electron_bindings.cc

@@ -31,15 +31,26 @@
 #include "shell/common/node_includes.h"
 #include "third_party/blink/renderer/platform/heap/process_heap.h"  // nogncheck
 
+#if !defined(MAS_BUILD)
+#include "shell/common/crash_keys.h"
+#endif
+
 namespace electron {
 
 namespace {
 
 // Called when there is a fatal error in V8, we just crash the process here so
 // we can get the stack trace.
-void FatalErrorCallback(const char* location, const char* message) {
+void V8FatalErrorCallback(const char* location, const char* message) {
   LOG(ERROR) << "Fatal error in V8: " << location << " " << message;
-  ElectronBindings::Crash();
+
+#if !defined(MAS_BUILD)
+  crash_keys::SetCrashKey("electron.v8-fatal.message", message);
+  crash_keys::SetCrashKey("electron.v8-fatal.location", location);
+#endif
+
+  volatile int* zero = nullptr;
+  *zero = 0;
 }
 
 }  // namespace
@@ -86,7 +97,7 @@ void ElectronBindings::BindProcess(v8::Isolate* isolate,
 
 void ElectronBindings::BindTo(v8::Isolate* isolate,
                               v8::Local<v8::Object> process) {
-  isolate->SetFatalErrorHandler(FatalErrorCallback);
+  isolate->SetFatalErrorHandler(V8FatalErrorCallback);
 
   gin_helper::Dictionary dict(isolate, process);
   BindProcess(isolate, &dict, metrics_.get());

+ 28 - 0
spec-main/api-crash-reporter-spec.ts

@@ -39,6 +39,8 @@ type CrashInfo = {
   globalParam: 'globalValue' | undefined
   addedThenRemoved: 'to-be-removed' | undefined
   longParam: string | undefined
+  'electron.v8-fatal.location': string | undefined
+  'electron.v8-fatal.message': string | undefined
 }
 
 function checkCrash (expectedProcessType: string, fields: CrashInfo) {
@@ -279,6 +281,32 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
         expect(crash.rendererSpecific).to.equal('rs');
         expect(crash.addedThenRemoved).to.be.undefined();
       });
+
+      it('contains v8 crash keys when a v8 crash occurs', async () => {
+        const { remotely } = await startRemoteControlApp();
+        const { port, waitForCrash } = await startServer();
+
+        await remotely((port: number) => {
+          require('electron').crashReporter.start({
+            submitURL: `http://127.0.0.1:${port}`,
+            ignoreSystemCrashHandler: true
+          });
+        }, [port]);
+
+        remotely(() => {
+          const { BrowserWindow } = require('electron');
+          const bw = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } });
+          bw.loadURL('about:blank');
+          bw.webContents.executeJavaScript('process.electronBinding(\'v8_util\').triggerFatalErrorForTesting()');
+        });
+
+        const crash = await waitForCrash();
+        expect(crash.prod).to.equal('Electron');
+        expect(crash._productName).to.equal('remote-control');
+        expect(crash.process_type).to.equal('renderer');
+        expect(crash['electron.v8-fatal.location']).to.equal('v8::Context::New()');
+        expect(crash['electron.v8-fatal.message']).to.equal('Circular extension dependency');
+      });
     });
   });
 

+ 1 - 0
typings/internal-ambient.d.ts

@@ -44,6 +44,7 @@ declare namespace NodeJS {
     weaklyTrackValue(value: any): void;
     clearWeaklyTrackedValues(): void;
     getWeaklyTrackedValues(): any[];
+    triggerFatalErrorForTesting(): void;
   }
 
   interface Process {