diff --git a/atom/common/api/atom_api_crash_reporter.cc b/atom/common/api/atom_api_crash_reporter.cc index aaaf200b32f3..0edd787e558c 100644 --- a/atom/common/api/atom_api_crash_reporter.cc +++ b/atom/common/api/atom_api_crash_reporter.cc @@ -31,19 +31,27 @@ struct Converter { namespace { +void SetExtraParameter(const std::string& key, mate::Arguments* args) { + std::string value; + if (args->GetNext(&value)) + CrashReporter::GetInstance()->SetExtraParameter(key, value); + else + CrashReporter::GetInstance()->RemoveExtraParameter(key); +} + void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); - auto report = base::Unretained(CrashReporter::GetInstance()); - dict.SetMethod("start", - base::Bind(&CrashReporter::Start, report)); - dict.SetMethod("_getUploadedReports", - base::Bind(&CrashReporter::GetUploadedReports, report)); - dict.SetMethod("_setUploadToServer", - base::Bind(&CrashReporter::SetUploadToServer, report)); - dict.SetMethod("_getUploadToServer", - base::Bind(&CrashReporter::GetUploadToServer, report)); + auto reporter = base::Unretained(CrashReporter::GetInstance()); + dict.SetMethod("start", base::Bind(&CrashReporter::Start, reporter)); + dict.SetMethod("setExtraParameter", &SetExtraParameter); + dict.SetMethod("getUploadedReports", + base::Bind(&CrashReporter::GetUploadedReports, reporter)); + dict.SetMethod("setUploadToServer", + base::Bind(&CrashReporter::SetUploadToServer, reporter)); + dict.SetMethod("getUploadToServer", + base::Bind(&CrashReporter::GetUploadToServer, reporter)); } } // namespace diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index f8a5f5e29ecb..d901f83fa49f 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -86,6 +86,13 @@ void CrashReporter::InitBreakpad(const std::string& product_name, void CrashReporter::SetUploadParameters() { } +void CrashReporter::SetExtraParameter(const std::string& key, + const std::string& value) { +} + +void CrashReporter::RemoveExtraParameter(const std::string& key) { +} + #if defined(OS_MACOSX) && defined(MAS_BUILD) // static CrashReporter* CrashReporter::GetInstance() { diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index c564527109b8..2bdcd9b02d3b 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -37,6 +37,9 @@ class CrashReporter { virtual void SetUploadToServer(bool upload_to_server); virtual bool GetUploadToServer(); + virtual void SetExtraParameter(const std::string& key, + const std::string& value); + virtual void RemoveExtraParameter(const std::string& key); protected: CrashReporter(); diff --git a/atom/common/crash_reporter/crash_reporter_linux.cc b/atom/common/crash_reporter/crash_reporter_linux.cc index 50e1c5ec862c..9020977716ec 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.cc +++ b/atom/common/crash_reporter/crash_reporter_linux.cc @@ -64,13 +64,15 @@ void CrashReporterLinux::InitBreakpad(const std::string& product_name, bool skip_system_crash_handler) { EnableCrashDumping(crashes_dir); - crash_keys_.SetKeyValue("prod", ATOM_PRODUCT_NAME); - crash_keys_.SetKeyValue("ver", version.c_str()); + crash_keys_.reset(new CrashKeyStorage()); + + crash_keys_->SetKeyValue("prod", ATOM_PRODUCT_NAME); + crash_keys_->SetKeyValue("ver", version.c_str()); upload_url_ = submit_url; for (StringMap::const_iterator iter = upload_parameters_.begin(); iter != upload_parameters_.end(); ++iter) - crash_keys_.SetKeyValue(iter->first.c_str(), iter->second.c_str()); + crash_keys_->SetKeyValue(iter->first.c_str(), iter->second.c_str()); } void CrashReporterLinux::SetUploadParameters() { @@ -120,7 +122,7 @@ bool CrashReporterLinux::CrashDone(const MinidumpDescriptor& minidump, info.oom_size = base::g_oom_size; info.pid = self->pid_; info.upload_url = self->upload_url_.c_str(); - info.crash_keys = &self->crash_keys_; + info.crash_keys = self->crash_keys_.get(); HandleCrashDump(info); return true; } diff --git a/atom/common/crash_reporter/crash_reporter_linux.h b/atom/common/crash_reporter/crash_reporter_linux.h index 846e1f1b0a91..997caf1c271e 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.h +++ b/atom/common/crash_reporter/crash_reporter_linux.h @@ -49,7 +49,7 @@ class CrashReporterLinux : public CrashReporter { const bool succeeded); std::unique_ptr breakpad_; - CrashKeyStorage crash_keys_; + std::unique_ptr crash_keys_; uint64_t process_start_time_; pid_t pid_; diff --git a/atom/common/crash_reporter/crash_reporter_mac.h b/atom/common/crash_reporter/crash_reporter_mac.h index 9066f2017bb2..583c99a86e9c 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.h +++ b/atom/common/crash_reporter/crash_reporter_mac.h @@ -34,6 +34,9 @@ class CrashReporterMac : public CrashReporter { void SetUploadParameters() override; void SetUploadToServer(bool upload_to_server) override; bool GetUploadToServer() override; + void SetExtraParameter(const std::string& key, + const std::string& value) override; + void RemoveExtraParameter(const std::string& key) override; private: friend struct base::DefaultSingletonTraits; diff --git a/atom/common/crash_reporter/crash_reporter_mac.mm b/atom/common/crash_reporter/crash_reporter_mac.mm index cdaa5d2f52e6..4b59be5dfca8 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.mm +++ b/atom/common/crash_reporter/crash_reporter_mac.mm @@ -100,6 +100,21 @@ void CrashReporterMac::SetCrashKeyValue(const base::StringPiece& key, simple_string_dictionary_->SetKeyValue(key.data(), value.data()); } +void CrashReporterMac::SetExtraParameter(const std::string& key, + const std::string& value) { + if (simple_string_dictionary_) + SetCrashKeyValue(key, value); + else + upload_parameters_[key] = value; +} + +void CrashReporterMac::RemoveExtraParameter(const std::string& key) { + if (simple_string_dictionary_) + simple_string_dictionary_->RemoveKey(key.data()); + else + upload_parameters_.erase(key); +} + std::vector CrashReporterMac::GetUploadedReports(const base::FilePath& crashes_dir) { std::vector uploaded_reports; diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 3e93de4a25b4..818aa8e7b54f 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -44,14 +44,14 @@ The `crashReporter` module has the following methods: Default is `true`. * `ignoreSystemCrashHandler` Boolean (optional) - Default is `false`. * `extra` Object (optional) - An object you can define that will be sent along with the - report. Only string properties are sent correctly, Nested objects are not + report. Only string properties are sent correctly. Nested objects are not supported. You are required to call this method before using any other `crashReporter` APIs and in each process (main/renderer) from which you want to collect crash reports. You can pass different options to `crashReporter.start` when calling from different processes. -**Note** Child processes created via the `child_process` module will not have access to the Electron modules. +**Note** Child processes created via the `child_process` module will not have access to the Electron modules. Therefore, to collect crash reports from them, use `process.crashReporter.start` instead. Pass the same options as above along with an additional one called `crashesDirectory` that should point to a directory to store the crash reports temporarily. You can test this out by calling `process.crash()` to crash the child process. @@ -60,6 +60,10 @@ reports temporarily. You can test this out by calling `process.crash()` to crash This will start the process that will monitor and send the crash reports. Replace `submitURL`, `productName` and `crashesDirectory` with appropriate values. +**Note:** If you need send additional/updated `extra` parameters after your +first call `start` you can call `setExtraParameter` on macOS or call `start` +again with the new/updated `extra` parameters on Linux and Windows. + ```js const args = [ `--reporter-url=${submitURL}`, @@ -111,6 +115,18 @@ called before `start` is called. **Note:** This API can only be called from the main process. +### `crashReporter.setExtraParameter(key, value)` _macOS_ + +* `key` String - Parameter key. +* `value` String - Parameter value. Specifying `null` or `undefined` will + remove the key from the extra parameters. + +Set an extra parameter to set be sent with the crash report. The values +specified here will be sent in addition to any values set via the `extra` option +when `start` was called. This API is only available on macOS, if you need to +add/update extra parameters on Linux and Windows after your first call to +`start` you can call `start` again with the updated `extra` options. + ## Crash Report Payload The crash reporter will send the following data to the `submitURL` as diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index c5a462cf5110..658622e8f5a7 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -75,7 +75,7 @@ class CrashReporter { } getUploadedReports () { - return binding._getUploadedReports(this.getCrashesDirectory()) + return binding.getUploadedReports(this.getCrashesDirectory()) } getCrashesDirectory () { @@ -104,7 +104,7 @@ class CrashReporter { getUploadToServer () { if (process.type === 'browser') { - return binding._getUploadToServer() + return binding.getUploadToServer() } else { throw new Error('getUploadToServer can only be called from the main process') } @@ -112,11 +112,15 @@ class CrashReporter { setUploadToServer (uploadToServer) { if (process.type === 'browser') { - return binding._setUploadToServer(uploadToServer) + return binding.setUploadToServer(uploadToServer) } else { throw new Error('setUploadToServer can only be called from the main process') } } + + setExtraParameter (key, value) { + binding.setExtraParameter(key, value) + } } module.exports = new CrashReporter() diff --git a/spec/api-crash-reporter-spec.js b/spec/api-crash-reporter-spec.js index 0871670de370..c269cfe1324b 100644 --- a/spec/api-crash-reporter-spec.js +++ b/spec/api-crash-reporter-spec.js @@ -35,7 +35,7 @@ describe('crashReporter module', function () { } it('should send minidump when renderer crashes', function (done) { - if (process.platform !== 'darwin') return done() + if (process.env.APPVEYOR === 'True') return done() if (process.env.TRAVIS === 'true') return done() this.timeout(120000) @@ -55,7 +55,7 @@ describe('crashReporter module', function () { }) it('should send minidump when node processes crash', function (done) { - if (process.platform !== 'darwin') return done() + if (process.env.APPVEYOR === 'True') return done() if (process.env.TRAVIS === 'true') return done() this.timeout(120000) @@ -72,6 +72,26 @@ describe('crashReporter module', function () { }) }) + it('should send minidump with updated extra parameters', function (done) { + if (process.env.APPVEYOR === 'True') return done() + if (process.env.TRAVIS === 'true') return done() + + this.timeout(10000) + + startServer({ + callback (port) { + const crashUrl = url.format({ + protocol: 'file', + pathname: path.join(fixtures, 'api', 'crash-restart.html'), + search: '?port=' + port + }) + w.loadURL(crashUrl) + }, + processType: 'renderer', + done: done + }) + }) + describe('.start(options)', function () { it('requires that the companyName and submitURL options be specified', function () { assert.throws(function () { @@ -111,7 +131,7 @@ describe('crashReporter module', function () { crashReporter.start({ companyName: 'Umbrella Corporation', submitURL: 'http://127.0.0.1/crashes', - autoSubmit: true + uploadToServer: true }) assert.equal(crashReporter.getUploadToServer(), true) crashReporter.setUploadToServer(false) @@ -155,6 +175,7 @@ const startServer = ({callback, processType, done}) => { assert.equal(fields.platform, process.platform) assert.equal(fields.extra1, 'extra1') assert.equal(fields.extra2, 'extra2') + assert.equal(fields.extra3, undefined) assert.equal(fields._productName, 'Zombies') assert.equal(fields._companyName, 'Umbrella Corporation') assert.equal(fields._version, app.getVersion()) @@ -165,6 +186,7 @@ const startServer = ({callback, processType, done}) => { assert.equal(crashReporter.getLastCrashReport().id, reportId) assert.notEqual(crashReporter.getUploadedReports().length, 0) assert.equal(crashReporter.getUploadedReports()[0].id, reportId) + req.socket.destroy() done() }, done) }) diff --git a/spec/fixtures/api/crash-restart.html b/spec/fixtures/api/crash-restart.html new file mode 100644 index 000000000000..2f55c539bbd8 --- /dev/null +++ b/spec/fixtures/api/crash-restart.html @@ -0,0 +1,46 @@ + + + + + diff --git a/spec/fixtures/api/crash.html b/spec/fixtures/api/crash.html index c1fb621426af..2b1cea6e9af3 100644 --- a/spec/fixtures/api/crash.html +++ b/spec/fixtures/api/crash.html @@ -7,7 +7,7 @@ crashReporter.start({ productName: 'Zombies', companyName: 'Umbrella Corporation', submitURL: 'http://127.0.0.1:' + port, - autoSubmit: true, + uploadToServer: true, ignoreSystemCrashHandler: true, extra: { 'extra1': 'extra1',