Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: send guid with linux crashes #24897

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions shell/browser/api/electron_api_crash_reporter.cc
Expand Up @@ -40,7 +40,11 @@
#endif

#if defined(OS_LINUX)
#include "base/containers/span.h"
#include "base/files/file_util.h"
#include "base/guid.h"
#include "components/crash/core/app/breakpad_linux.h"
#include "components/crash/core/common/crash_keys.h"
#include "v8/include/v8-wasm-trap-handler-posix.h"
#include "v8/include/v8.h"
#endif
Expand Down Expand Up @@ -81,6 +85,40 @@ bool IsCrashReporterEnabled() {
const std::map<std::string, std::string>& GetGlobalCrashKeys() {
return GetGlobalCrashKeysMutable();
}

base::FilePath GetClientIdPath() {
base::FilePath path;
base::PathService::Get(electron::DIR_CRASH_DUMPS, &path);
return path.Append("client_id");
}

std::string ReadClientId() {
base::ThreadRestrictions::ScopedAllowIO allow_io;
std::string client_id;
// "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".length == 36
if (!base::ReadFileToStringWithMaxSize(GetClientIdPath(), &client_id, 36) ||
client_id.size() != 36)
return std::string();
return client_id;
}

void WriteClientId(const std::string& client_id) {
DCHECK_EQ(client_id.size(), 36u);
base::ThreadRestrictions::ScopedAllowIO allow_io;
base::WriteFile(GetClientIdPath(), client_id);
}

std::string GetClientId() {
static base::NoDestructor<std::string> client_id;
if (!client_id->empty())
return *client_id;
*client_id = ReadClientId();
if (client_id->empty()) {
*client_id = base::GenerateGUID();
WriteClientId(*client_id);
}
return *client_id;
}
#endif

void Start(const std::string& submit_url,
Expand All @@ -107,6 +145,7 @@ void Start(const std::string& submit_url,
? "node"
: command_line->GetSwitchValueASCII(::switches::kProcessType);
#if defined(OS_LINUX)
::crash_keys::SetMetricsClientIdFromGUID(GetClientId());
auto& global_crash_keys = GetGlobalCrashKeysMutable();
for (const auto& pair : global_extra) {
global_crash_keys[pair.first] = pair.second;
Expand Down
1 change: 1 addition & 0 deletions shell/browser/api/electron_api_crash_reporter.h
Expand Up @@ -19,6 +19,7 @@ bool IsCrashReporterEnabled();

#if defined(OS_LINUX)
const std::map<std::string, std::string>& GetGlobalCrashKeys();
std::string GetClientId();
#endif

// JS bindings API; exposed publicly because it's also called from node_main.cc
Expand Down
6 changes: 4 additions & 2 deletions shell/browser/electron_browser_client.cc
Expand Up @@ -721,8 +721,10 @@ void ElectronBrowserClient::AppendExtraCommandLineSwitches(
bool enable_crash_reporter = false;
enable_crash_reporter = breakpad::IsCrashReporterEnabled();
if (enable_crash_reporter) {
command_line->AppendSwitch(::switches::kEnableCrashReporter);
std::string switch_value;
std::string switch_value =
api::crash_reporter::GetClientId() + ",no_channel";
command_line->AppendSwitchASCII(::switches::kEnableCrashReporter,
switch_value);
for (const auto& pair : api::crash_reporter::GetGlobalCrashKeys()) {
if (!switch_value.empty())
switch_value += ",";
Expand Down
30 changes: 30 additions & 0 deletions spec-main/api-crash-reporter-spec.ts
Expand Up @@ -33,6 +33,7 @@ type CrashInfo = {
_productName: string
_version: string
upload_file_minidump: Buffer // eslint-disable-line camelcase
guid: string
mainProcessSpecific: 'mps' | undefined
rendererSpecific: 'rs' | undefined
globalParam: 'globalValue' | undefined
Expand Down Expand Up @@ -229,6 +230,35 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
expect(crash.rendererSpecific).to.be.undefined();
});

describe('with guid', () => {
for (const processType of ['main', 'renderer', 'sandboxed-renderer']) {
it(`when ${processType} crashes`, async () => {
const { port, waitForCrash } = await startServer();
runCrashApp(processType, port);
const crash = await waitForCrash();
expect(crash.guid).to.be.a('string');
});
}

it('is a consistent id', async () => {
let crash1Guid;
let crash2Guid;
{
const { port, waitForCrash } = await startServer();
runCrashApp('main', port);
const crash = await waitForCrash();
crash1Guid = crash.guid;
}
{
const { port, waitForCrash } = await startServer();
runCrashApp('main', port);
const crash = await waitForCrash();
crash2Guid = crash.guid;
}
expect(crash2Guid).to.equal(crash1Guid);
});
});

describe('with extra parameters', () => {
it('when renderer crashes', async () => {
const { port, waitForCrash } = await startServer();
Expand Down