Skip to content

Commit

Permalink
fix: save crash reports locally when uploadToServer: false on linux (#…
Browse files Browse the repository at this point in the history 
…24778) (#24788)

* fix: generate dumps under crashDumps folder in linux

* Update spec-main/api-crash-reporter-spec.ts

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
  • Loading branch information
@deepak1556 @nornagon
deepak1556 and nornagon committed Jul 31, 2020
1 parent f65756b commit 0107cab
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 16 deletions.
12 changes: 10 additions & 2 deletions shell/app/electron_crash_reporter_client.cc
View file
Expand Up @@ -11,6 +11,7 @@
#include "base/command_line.h"
#include "base/environment.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/strings/string_split.h"
Expand Down Expand Up @@ -147,7 +148,14 @@ bool ElectronCrashReporterClient::GetCrashDumpLocation(
#else
bool ElectronCrashReporterClient::GetCrashDumpLocation(
base::FilePath* crash_dir) {
return base::PathService::Get(electron::DIR_CRASH_DUMPS, crash_dir);
bool result = base::PathService::Get(electron::DIR_CRASH_DUMPS, crash_dir);
{
base::ThreadRestrictions::ScopedAllowIO allow_io;
if (result && !base::PathExists(*crash_dir)) {
return base::CreateDirectory(*crash_dir);
}
}
return result;
}
#endif

Expand All @@ -159,7 +167,7 @@ bool ElectronCrashReporterClient::GetCrashMetricsLocation(
#endif // OS_MACOSX || OS_LINUX

bool ElectronCrashReporterClient::IsRunningUnattended() {
return false;
return !collect_stats_consent_;
}

bool ElectronCrashReporterClient::GetCollectStatsConsent() {
Expand Down
4 changes: 3 additions & 1 deletion shell/browser/electron_browser_client.cc
View file
Expand Up @@ -60,6 +60,7 @@
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/resource_request_body.h"
#include "services/service_manager/public/cpp/binder_map.h"
#include "shell/app/electron_crash_reporter_client.h"
#include "shell/app/manifests.h"
#include "shell/browser/api/electron_api_app.h"
#include "shell/browser/api/electron_api_crash_reporter.h"
Expand Down Expand Up @@ -284,8 +285,9 @@ breakpad::CrashHandlerHostLinux* CreateCrashHandlerHost(
base::PathService::Get(electron::DIR_CRASH_DUMPS, &dumps_path);
{
ANNOTATE_SCOPED_MEMORY_LEAK;
bool upload = ElectronCrashReporterClient::Get()->GetCollectStatsConsent();
breakpad::CrashHandlerHostLinux* crash_handler =
new breakpad::CrashHandlerHostLinux(process_type, dumps_path, true);
new breakpad::CrashHandlerHostLinux(process_type, dumps_path, upload);
crash_handler->StartUploaderThread();
return crash_handler;
}
Expand Down
55 changes: 42 additions & 13 deletions spec-main/api-crash-reporter-spec.ts
View file
Expand Up @@ -260,7 +260,7 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
require('electron').crashReporter.start({
submitURL: `http://127.0.0.1:${port}`,
ignoreSystemCrashHandler: true,
extra: { 'longParam': 'a'.repeat(130) }
extra: { longParam: 'a'.repeat(130) }
});
setTimeout(() => process.crash());
}, port);
Expand Down Expand Up @@ -438,7 +438,7 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
await remotely(() => {
require('electron').crashReporter.start({
submitURL: 'http://127.0.0.1',
extra: { 'extra1': 'hi' }
extra: { extra1: 'hi' }
});
});
const parameters = await remotely(() => require('electron').crashReporter.getParameters());
Expand Down Expand Up @@ -471,8 +471,8 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
crashReporter.start({ submitURL: 'http://' });
const bw = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } });
bw.loadURL('about:blank');
await bw.webContents.executeJavaScript(`require('electron').crashReporter.addExtraParameter('hello', 'world')`);
return bw.webContents.executeJavaScript(`require('electron').crashReporter.getParameters()`);
await bw.webContents.executeJavaScript('require(\'electron\').crashReporter.addExtraParameter(\'hello\', \'world\')');
return bw.webContents.executeJavaScript('require(\'electron\').crashReporter.getParameters()');
});
if (process.platform === 'linux') {
// On Linux, 'getParameters' will also include the global parameters,
Expand Down Expand Up @@ -542,24 +542,38 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
}
}

for (const crashingProcess of ['main', 'renderer', 'sandboxed-renderer', 'node']) {
// TODO(nornagon): breakpad on linux disables itself when uploadToServer
// is false, so we should figure out a different way to test the crash
// dump dir on linux.
ifdescribe(process.platform !== 'linux')(`when ${crashingProcess} crashes`, () => {
const processList = process.platform === 'linux' ? ['main', 'renderer', 'sandboxed-renderer']
: ['main', 'renderer', 'sandboxed-renderer', 'node'];
for (const crashingProcess of processList) {
describe(`when ${crashingProcess} crashes`, () => {
it('stores crashes in the crash dump directory when uploadToServer: false', async () => {
const { remotely } = await startRemoteControlApp();
const crashesDir = await remotely(() => {
const { crashReporter } = require('electron');
crashReporter.start({ submitURL: 'http://127.0.0.1', uploadToServer: false, ignoreSystemCrashHandler: true });
return crashReporter.getCrashesDirectory();
});
const reportsDir = process.platform === 'darwin' ? path.join(crashesDir, 'completed') : path.join(crashesDir, 'reports');
let reportsDir = crashesDir;
if (process.platform === 'darwin') {
reportsDir = path.join(crashesDir, 'completed');
} else if (process.platform === 'win32') {
reportsDir = path.join(crashesDir, 'reports');
}
const newFileAppeared = waitForNewFileInDir(reportsDir);
crash(crashingProcess, remotely);
const newFiles = await newFileAppeared;
expect(newFiles.length).to.be.greaterThan(0);
expect(newFiles[0]).to.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.dmp$/);
if (process.platform === 'linux') {
if (crashingProcess === 'main') {
expect(newFiles[0]).to.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{8}-[0-9a-f]{8}\.dmp$/);
} else {
const process = crashingProcess === 'sandboxed-renderer' ? 'renderer' : crashingProcess;
const regex = RegExp(`chromium-${process}-minidump-[0-9a-f]{16}.dmp`);
expect(newFiles[0]).to.match(regex);
}
} else {
expect(newFiles[0]).to.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.dmp$/);
}
});

it('respects an overridden crash dump directory', async () => {
Expand All @@ -573,12 +587,27 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
}, crashesDir);
expect(remoteCrashesDir).to.equal(crashesDir);

const reportsDir = process.platform === 'darwin' ? path.join(crashesDir, 'completed') : path.join(crashesDir, 'reports');
let reportsDir = crashesDir;
if (process.platform === 'darwin') {
reportsDir = path.join(crashesDir, 'completed');
} else if (process.platform === 'win32') {
reportsDir = path.join(crashesDir, 'reports');
}
const newFileAppeared = waitForNewFileInDir(reportsDir);
crash(crashingProcess, remotely);
const newFiles = await newFileAppeared;
expect(newFiles.length).to.be.greaterThan(0);
expect(newFiles[0]).to.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.dmp$/);
if (process.platform === 'linux') {
if (crashingProcess === 'main') {
expect(newFiles[0]).to.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{8}-[0-9a-f]{8}\.dmp$/);
} else {
const process = crashingProcess !== 'sandboxed-renderer' ? crashingProcess : 'renderer';
const regex = RegExp(`chromium-${process}-minidump-[0-9a-f]{16}.dmp`);
expect(newFiles[0]).to.match(regex);
}
} else {
expect(newFiles[0]).to.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.dmp$/);
}
});
});
}
Expand Down

0 comments on commit 0107cab

Please sign in to comment.