Skip to content

Commit

Permalink
feat: add V8 crash information to crashReporter
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Jul 29, 2020
1 parent 38fafe4 commit bc89ff5
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 0 deletions.
11 changes: 11 additions & 0 deletions shell/common/api/electron_api_v8_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ void RequestGarbageCollectionForTesting(v8::Isolate* isolate) {
v8::Isolate::GarbageCollectionType::kFullGarbageCollection);
}

// 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);
}

bool IsSameOrigin(const GURL& l, const GURL& r) {
return url::Origin::Create(l).IsSameOriginWith(url::Origin::Create(r));
}
Expand Down Expand Up @@ -143,6 +153,7 @@ void Initialize(v8::Local<v8::Object> exports,
dict.SetMethod("requestGarbageCollectionForTesting",
&RequestGarbageCollectionForTesting);
dict.SetMethod("isSameOrigin", &IsSameOrigin);
dict.SetMethod("triggerFatalErrorForTesting", &TriggerFatalErrorForTesting);
#ifdef DCHECK_IS_ON
dict.SetMethod("getWeaklyTrackedValues", &GetWeaklyTrackedValues);
dict.SetMethod("clearWeaklyTrackedValues", &ClearWeaklyTrackedValues);
Expand Down
18 changes: 18 additions & 0 deletions shell/common/node_bindings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/common/content_paths.h"
#include "electron/buildflags/buildflags.h"
#include "shell/common/api/electron_bindings.h"
#include "shell/common/electron_command_line.h"
#include "shell/common/gin_converters/file_path_converter.h"
#include "shell/common/gin_helper/dictionary.h"
Expand All @@ -33,6 +34,10 @@
#include "shell/common/mac/main_application_bundle.h"
#include "shell/common/node_includes.h"

#if !defined(MAS_BUILD)
#include "shell/common/crash_keys.h"
#endif

#define ELECTRON_BUILTIN_MODULES(V) \
V(electron_browser_app) \
V(electron_browser_auto_updater) \
Expand Down Expand Up @@ -132,6 +137,15 @@ bool IsPackagedApp() {
#endif
}

void FatalErrorCallback(const char* location, const char* message) {
LOG(ERROR) << "Fatal error in V8: " << location << " " << message;

electron::crash_keys::SetCrashKey("electron.v8-fatal.message", message);
electron::crash_keys::SetCrashKey("electron.v8-fatal.location", location);

electron::ElectronBindings::Crash();
}

// Initialize Node.js cli options to pass to Node.js
// See https://nodejs.org/api/cli.html#cli_options
void SetNodeCliFlags() {
Expand Down Expand Up @@ -397,6 +411,10 @@ node::Environment* NodeBindings::CreateEnvironment(

node::IsolateSettings is;

// Use a custom fatal error callback to allow us to add
// crash message and location to CrashReports.
is.fatal_error_callback = FatalErrorCallback;

if (browser_env_ == BrowserEnvironment::BROWSER) {
// Node.js requires that microtask checkpoints be explicitly invoked.
is.policy = v8::MicrotasksPolicy::kExplicit;
Expand Down
28 changes: 28 additions & 0 deletions spec-main/api-crash-reporter-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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) {
Expand Down Expand Up @@ -192,6 +194,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._linkedBinding(\'electron_common_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');
});
});
});

Expand Down
1 change: 1 addition & 0 deletions typings/internal-ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ declare namespace NodeJS {
clearWeaklyTrackedValues(): void;
getWeaklyTrackedValues(): any[];
addRemoteObjectRef(contextId: string, id: number): void;
triggerFatalErrorForTesting(): void;
}

type AsarFileInfo = {
Expand Down

0 comments on commit bc89ff5

Please sign in to comment.