From ab24a1e36dcdc102cf6c066e2ab345549ad36be2 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Mon, 18 Jun 2018 18:45:58 -0700 Subject: [PATCH] feat: netLog API for dynamic logging control (#13068) * Introduce `net.{start|stop}Logging()` - Slight regression right now as Electron won't automatically start logging net-logs at launch, will soon be fixed - To implement callback for async controls * Add `net.isLogging` & optional callback param for `net.stopLogging()` * Fix small regression on --log-net-log --log-net-log should work again * Error on empty file path * Only start with valid file path * Remove unused var * Allow setting log file path before URLRequestContextGetter starts logging * Add net log tests * Remove redundant checks * Use brightray::NetLog * Clean up code * Should automatically stop listening * :art: Attempt to fix styles * Only run non-null callback * Dump file to tmpdir * Simplify net log spec Spawned Electron process on Linux CI can fail to launch * Separate netLog module * Remove net logging test from net spec * Add tests for netLog * Fix header guard * Clean up code * Add netLog.currentlyLoggingPath * Callback with filepath * Add test for case when only .stopLogging() is called * Add docs * Reintroduce error on invalid arg * Update copyright * Update error message * Juggle file path string types --- atom/browser/api/atom_api_net_log.cc | 90 ++++++++++++++ atom/browser/api/atom_api_net_log.h | 42 +++++++ atom/common/node_bindings.cc | 1 + brightray/browser/browser_client.cc | 3 +- brightray/browser/browser_client.h | 2 +- brightray/browser/net_log.cc | 59 ++++++++-- brightray/browser/net_log.h | 10 ++ docs/README.md | 1 + docs/api/net-log.md | 42 +++++++ filenames.gypi | 3 + lib/browser/api/module-list.js | 1 + lib/browser/api/net-log.js | 16 +++ spec/api-net-log-spec.js | 156 +++++++++++++++++++++++++ spec/api-net-spec.js | 2 +- spec/fixtures/api/net-log/main.js | 33 ++++++ spec/fixtures/api/net-log/package.json | 4 + 16 files changed, 453 insertions(+), 12 deletions(-) create mode 100644 atom/browser/api/atom_api_net_log.cc create mode 100644 atom/browser/api/atom_api_net_log.h create mode 100644 docs/api/net-log.md create mode 100644 lib/browser/api/net-log.js create mode 100644 spec/api-net-log-spec.js create mode 100644 spec/fixtures/api/net-log/main.js create mode 100644 spec/fixtures/api/net-log/package.json diff --git a/atom/browser/api/atom_api_net_log.cc b/atom/browser/api/atom_api_net_log.cc new file mode 100644 index 00000000000..299ef76f83a --- /dev/null +++ b/atom/browser/api/atom_api_net_log.cc @@ -0,0 +1,90 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/api/atom_api_net_log.h" +#include "atom/browser/atom_browser_client.h" +#include "atom/common/native_mate_converters/callback.h" +#include "atom/common/native_mate_converters/file_path_converter.h" +#include "base/callback.h" +#include "content/public/common/content_switches.h" +#include "native_mate/dictionary.h" +#include "native_mate/handle.h" + +#include "atom/common/node_includes.h" + +namespace atom { + +namespace api { + +NetLog::NetLog(v8::Isolate* isolate) { + Init(isolate); + + net_log_ = atom::AtomBrowserClient::Get()->GetNetLog(); +} + +NetLog::~NetLog() {} + +// static +v8::Local NetLog::Create(v8::Isolate* isolate) { + return mate::CreateHandle(isolate, new NetLog(isolate)).ToV8(); +} + +void NetLog::StartLogging(mate::Arguments* args) { + base::FilePath log_path; + if (!args->GetNext(&log_path) || log_path.empty()) { + args->ThrowError("The first parameter must be a valid string"); + return; + } + + net_log_->StartDynamicLogging(log_path); +} + +bool NetLog::IsCurrentlyLogging() { + return net_log_->IsDynamicLogging(); +} + +base::FilePath::StringType NetLog::GetCurrentlyLoggingPath() { + return net_log_->GetDynamicLoggingPath().value(); +} + +void NetLog::StopLogging(mate::Arguments* args) { + base::OnceClosure callback; + args->GetNext(&callback); + + net_log_->StopDynamicLogging(std::move(callback)); +} + +// static +void NetLog::BuildPrototype(v8::Isolate* isolate, + v8::Local prototype) { + prototype->SetClassName(mate::StringToV8(isolate, "NetLog")); + mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) + .SetProperty("currentlyLogging", &NetLog::IsCurrentlyLogging) + .SetProperty("currentlyLoggingPath", &NetLog::GetCurrentlyLoggingPath) + .SetMethod("startLogging", &NetLog::StartLogging) + .SetMethod("_stopLogging", &NetLog::StopLogging); +} + +} // namespace api + +} // namespace atom + +namespace { + +using atom::api::NetLog; + +void Initialize(v8::Local exports, + v8::Local unused, + v8::Local context, + void* priv) { + v8::Isolate* isolate = context->GetIsolate(); + + mate::Dictionary dict(isolate, exports); + dict.Set("netLog", NetLog::Create(isolate)); + dict.Set("NetLog", NetLog::GetConstructor(isolate)->GetFunction()); +} + +} // namespace + +NODE_BUILTIN_MODULE_CONTEXT_AWARE(atom_browser_net_log, Initialize) diff --git a/atom/browser/api/atom_api_net_log.h b/atom/browser/api/atom_api_net_log.h new file mode 100644 index 00000000000..dd68ac306c9 --- /dev/null +++ b/atom/browser/api/atom_api_net_log.h @@ -0,0 +1,42 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_API_ATOM_API_NET_LOG_H_ +#define ATOM_BROWSER_API_ATOM_API_NET_LOG_H_ + +#include +#include "brightray/browser/net_log.h" +#include "native_mate/wrappable.h" + +namespace atom { + +namespace api { + +class NetLog : public mate::Wrappable { + public: + static v8::Local Create(v8::Isolate* isolate); + + static void BuildPrototype(v8::Isolate* isolate, + v8::Local prototype); + + void StartLogging(mate::Arguments* args); + bool IsCurrentlyLogging(); + base::FilePath::StringType GetCurrentlyLoggingPath(); + void StopLogging(mate::Arguments* args); + + protected: + explicit NetLog(v8::Isolate* isolate); + ~NetLog() override; + + private: + brightray::NetLog* net_log_; + + DISALLOW_COPY_AND_ASSIGN(NetLog); +}; + +} // namespace api + +} // namespace atom + +#endif // ATOM_BROWSER_API_ATOM_API_NET_LOG_H_ diff --git a/atom/common/node_bindings.cc b/atom/common/node_bindings.cc index 20dfe412cb7..a26747cf03c 100644 --- a/atom/common/node_bindings.cc +++ b/atom/common/node_bindings.cc @@ -39,6 +39,7 @@ V(atom_browser_in_app_purchase) \ V(atom_browser_menu) \ V(atom_browser_net) \ + V(atom_browser_net_log) \ V(atom_browser_power_monitor) \ V(atom_browser_power_save_blocker) \ V(atom_browser_protocol) \ diff --git a/brightray/browser/browser_client.cc b/brightray/browser/browser_client.cc index ea82524810c..b2b7d36301f 100644 --- a/brightray/browser/browser_client.cc +++ b/brightray/browser/browser_client.cc @@ -58,7 +58,6 @@ BrowserClient::BrowserClient() : browser_main_parts_(nullptr) { BrowserClient::~BrowserClient() {} - void BrowserClient::WebNotificationAllowed( int render_process_id, const base::Callback& callback) { @@ -107,7 +106,7 @@ void BrowserClient::GetAdditionalWebUISchemes( additional_schemes->push_back(content::kChromeDevToolsScheme); } -net::NetLog* BrowserClient::GetNetLog() { +NetLog* BrowserClient::GetNetLog() { return &net_log_; } diff --git a/brightray/browser/browser_client.h b/brightray/browser/browser_client.h index 5c8ecd397c7..643e493f43f 100644 --- a/brightray/browser/browser_client.h +++ b/brightray/browser/browser_client.h @@ -45,7 +45,7 @@ class BrowserClient : public content::ContentBrowserClient { std::vector* additional_schemes) override; void GetAdditionalWebUISchemes( std::vector* additional_schemes) override; - net::NetLog* GetNetLog() override; + NetLog* GetNetLog() override; base::FilePath GetDefaultDownloadDirectory() override; content::DevToolsManagerDelegate* GetDevToolsManagerDelegate() override; std::string GetApplicationLocale() override; diff --git a/brightray/browser/net_log.cc b/brightray/browser/net_log.cc index 05e7f92b0ae..c02237a7032 100644 --- a/brightray/browser/net_log.cc +++ b/brightray/browser/net_log.cc @@ -6,7 +6,6 @@ #include -#include "base/callback.h" #include "base/command_line.h" #include "base/files/file_path.h" #include "base/values.h" @@ -36,10 +35,8 @@ std::unique_ptr GetConstants() { NetLog::NetLog() {} NetLog::~NetLog() { - if (file_net_log_observer_) { - file_net_log_observer_->StopObserving(nullptr, base::Closure()); - file_net_log_observer_.reset(); - } + StopDynamicLogging(); + StopLogging(); } void NetLog::StartLogging() { @@ -47,9 +44,12 @@ void NetLog::StartLogging() { if (!command_line->HasSwitch(switches::kLogNetLog)) return; - base::FilePath log_path = - command_line->GetSwitchValuePath(switches::kLogNetLog); - std::unique_ptr constants(GetConstants()); + base::FilePath log_path; + log_path = command_line->GetSwitchValuePath(switches::kLogNetLog); + if (log_path.empty()) + return; + + std::unique_ptr constants(GetConstants()); // Net constants net::NetLogCaptureMode capture_mode = net::NetLogCaptureMode::Default(); file_net_log_observer_ = @@ -57,4 +57,47 @@ void NetLog::StartLogging() { file_net_log_observer_->StartObserving(this, capture_mode); } +void NetLog::StopLogging() { + if (!file_net_log_observer_) + return; + + file_net_log_observer_->StopObserving(nullptr, base::OnceClosure()); + file_net_log_observer_.reset(); +} + +void NetLog::StartDynamicLogging(const base::FilePath& log_path) { + if (dynamic_file_net_log_observer_ || log_path.empty()) + return; + + dynamic_file_net_log_path_ = log_path; + + std::unique_ptr constants(GetConstants()); // Net constants + net::NetLogCaptureMode capture_mode = net::NetLogCaptureMode::Default(); + + dynamic_file_net_log_observer_ = net::FileNetLogObserver::CreateUnbounded( + dynamic_file_net_log_path_, std::move(constants)); + dynamic_file_net_log_observer_->StartObserving(this, capture_mode); +} + +bool NetLog::IsDynamicLogging() { + return !!dynamic_file_net_log_observer_; +} + +base::FilePath NetLog::GetDynamicLoggingPath() { + return dynamic_file_net_log_path_; +} + +void NetLog::StopDynamicLogging(base::OnceClosure callback) { + if (!dynamic_file_net_log_observer_) { + if (callback) + std::move(callback).Run(); + return; + } + + dynamic_file_net_log_observer_->StopObserving(nullptr, std::move(callback)); + dynamic_file_net_log_observer_.reset(); + + dynamic_file_net_log_path_ = base::FilePath(); +} + } // namespace brightray diff --git a/brightray/browser/net_log.h b/brightray/browser/net_log.h index 0359767f074..ffeb85f78a9 100644 --- a/brightray/browser/net_log.h +++ b/brightray/browser/net_log.h @@ -5,6 +5,8 @@ #ifndef BRIGHTRAY_BROWSER_NET_LOG_H_ #define BRIGHTRAY_BROWSER_NET_LOG_H_ +#include "base/callback.h" +#include "base/files/file_path.h" #include "net/log/net_log.h" namespace net { @@ -19,10 +21,18 @@ class NetLog : public net::NetLog { ~NetLog() override; void StartLogging(); + void StopLogging(); + + void StartDynamicLogging(const base::FilePath& path); + bool IsDynamicLogging(); + base::FilePath GetDynamicLoggingPath(); + void StopDynamicLogging(base::OnceClosure callback = base::OnceClosure()); private: // This observer handles writing NetLogs. std::unique_ptr file_net_log_observer_; + std::unique_ptr dynamic_file_net_log_observer_; + base::FilePath dynamic_file_net_log_path_; DISALLOW_COPY_AND_ASSIGN(NetLog); }; diff --git a/docs/README.md b/docs/README.md index e412379d9e1..99766c7d59a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -129,6 +129,7 @@ These individual tutorials expand on topics discussed in the guide above. * [Menu](api/menu.md) * [MenuItem](api/menu-item.md) * [net](api/net.md) +* [netLog](api/netLog.md) * [powerMonitor](api/power-monitor.md) * [powerSaveBlocker](api/power-save-blocker.md) * [protocol](api/protocol.md) diff --git a/docs/api/net-log.md b/docs/api/net-log.md new file mode 100644 index 00000000000..5e82a83a14d --- /dev/null +++ b/docs/api/net-log.md @@ -0,0 +1,42 @@ +# netLog + +> Logging network events. + +Process: [Main](../glossary.md#main-process) + +```javascript +const {netLog} = require('electron') +console.log('Start recording net-logs') +netLog.startLogging('/path/to/net-log') +// After some network events +netLog.stopLogging(path => { + console.log('Net-logs written to', path) +}) +``` + +See [`--log-net-log`](chrome-command-line-switches.md#--log-net-logpath) to log network events throughout the app's lifecycle. + +## Methods + +### `netLog.startLogging(path)` + +* `path` String - File path to record network logs. + +Starts recording network events to `path`. + +### `netLog.stopLogging([callback])` + +* `callback` Function (optional) + * `path` String - File path to which network logs were recorded. + +Stops recording network events. If not called, net logging will automatically end when app quits. + +## Properties + +### `netLog.currentlyLogging` + +A `Boolean` property that indicates whether network logs are recorded. + +### `netLog.currentlyLoggingPath` + +A `String` property that returns the path to the current log file. diff --git a/filenames.gypi b/filenames.gypi index be5e128cf91..78f99c1428c 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -28,6 +28,7 @@ 'lib/browser/api/module-list.js', 'lib/browser/api/navigation-controller.js', 'lib/browser/api/net.js', + 'lib/browser/api/net-log.js', 'lib/browser/api/notification.js', 'lib/browser/api/power-monitor.js', 'lib/browser/api/power-save-blocker.js', @@ -134,6 +135,8 @@ 'atom/browser/api/atom_api_menu_views.h', 'atom/browser/api/atom_api_net.cc', 'atom/browser/api/atom_api_net.h', + 'atom/browser/api/atom_api_net_log.cc', + 'atom/browser/api/atom_api_net_log.h', 'atom/browser/api/atom_api_notification.cc', 'atom/browser/api/atom_api_notification.h', 'atom/browser/api/atom_api_power_monitor_mac.mm', diff --git a/lib/browser/api/module-list.js b/lib/browser/api/module-list.js index 7de59088531..c84a8ce29cf 100644 --- a/lib/browser/api/module-list.js +++ b/lib/browser/api/module-list.js @@ -16,6 +16,7 @@ module.exports = [ {name: 'Menu', file: 'menu'}, {name: 'MenuItem', file: 'menu-item'}, {name: 'net', file: 'net'}, + {name: 'netLog', file: 'net-log'}, {name: 'Notification', file: 'notification'}, {name: 'powerMonitor', file: 'power-monitor'}, {name: 'powerSaveBlocker', file: 'power-save-blocker'}, diff --git a/lib/browser/api/net-log.js b/lib/browser/api/net-log.js new file mode 100644 index 00000000000..09d5244fc2d --- /dev/null +++ b/lib/browser/api/net-log.js @@ -0,0 +1,16 @@ +'use strict' + +const {netLog, NetLog} = process.atomBinding('net_log') + +NetLog.prototype.stopLogging = function (callback) { + if (callback && typeof callback !== 'function') { + throw new Error('Invalid callback function') + } + + const path = this.currentlyLoggingPath + this._stopLogging(() => { + if (callback) callback(path) + }) +} + +module.exports = netLog diff --git a/spec/api-net-log-spec.js b/spec/api-net-log-spec.js new file mode 100644 index 00000000000..1e9ca66722d --- /dev/null +++ b/spec/api-net-log-spec.js @@ -0,0 +1,156 @@ +const assert = require('assert') +const http = require('http') +const fs = require('fs') +const os = require('os') +const path = require('path') +const ChildProcess = require('child_process') +const {remote} = require('electron') +const {netLog} = remote +const appPath = path.join(__dirname, 'fixtures', 'api', 'net-log') +const dumpFile = path.join(os.tmpdir(), 'net_log.json') +const dumpFileDynamic = path.join(os.tmpdir(), 'net_log_dynamic.json') + +const isCI = remote.getGlobal('isCi') + +describe('netLog module', () => { + let server + const connections = new Set() + + before((done) => { + server = http.createServer() + server.listen(0, '127.0.0.1', () => { + server.url = `http://127.0.0.1:${server.address().port}` + done() + }) + server.on('connection', (connection) => { + connections.add(connection) + connection.once('close', () => { + connections.delete(connection) + }) + }) + server.on('request', (request, response) => { + response.end() + }) + }) + + after((done) => { + for (const connection of connections) { + connection.destroy() + } + server.close(() => { + server = null + done() + }) + }) + + afterEach(() => { + try { + fs.unlinkSync(dumpFile) + fs.unlinkSync(dumpFileDynamic) + } catch (e) { + // Ignore error + } + }) + + it('should begin and end logging to file when .startLogging() and .stopLogging() is called', (done) => { + assert(!netLog.currentlyLogging) + assert.equal(netLog.currentlyLoggingPath, '') + + netLog.startLogging(dumpFileDynamic) + + assert(netLog.currentlyLogging) + assert.equal(netLog.currentlyLoggingPath, dumpFileDynamic) + + netLog.stopLogging((path) => { + assert(!netLog.currentlyLogging) + assert.equal(netLog.currentlyLoggingPath, '') + + assert.equal(path, dumpFileDynamic) + + assert(fs.existsSync(dumpFileDynamic)) + + done() + }) + }) + + it('should silence when .stopLogging() is called without calling .startLogging()', (done) => { + assert(!netLog.currentlyLogging) + assert.equal(netLog.currentlyLoggingPath, '') + + netLog.stopLogging((path) => { + assert(!netLog.currentlyLogging) + assert.equal(netLog.currentlyLoggingPath, '') + + assert.equal(path, '') + + done() + }) + }) + + // The following tests are skipped on Linux CI + + it('should begin and end logging automatically when --log-net-log is passed', (done) => { + if (isCI && process.platform === 'linux') { + done() + return + } + + let appProcess = ChildProcess.spawn(remote.process.execPath, + [appPath, `--log-net-log=${dumpFile}`], { + env: { + TEST_REQUEST_URL: server.url + } + }) + + appProcess.once('exit', () => { + assert(fs.existsSync(dumpFile)) + done() + }) + }) + + it('should begin and end logging automtically when --log-net-log is passed, and behave correctly when .startLogging() and .stopLogging() is called', (done) => { + if (isCI && process.platform === 'linux') { + done() + return + } + + let appProcess = ChildProcess.spawn(remote.process.execPath, + [appPath, `--log-net-log=${dumpFile}`], { + env: { + TEST_REQUEST_URL: server.url, + TEST_DUMP_FILE: dumpFileDynamic, + TEST_MANUAL_STOP: true + } + }) + + appProcess.stdout.on('data', (data) => { + console.log(data.toString()) + }) + + appProcess.once('exit', () => { + assert(fs.existsSync(dumpFile)) + assert(fs.existsSync(dumpFileDynamic)) + done() + }) + }) + + it('should end logging automatically when only .startLogging() is called', (done) => { + if (isCI && process.platform === 'linux') { + done() + return + } + + let appProcess = ChildProcess.spawn(remote.process.execPath, + [appPath], { + env: { + TEST_REQUEST_URL: server.url, + TEST_DUMP_FILE: dumpFileDynamic + } + }) + + appProcess.once('exit', () => { + assert(fs.existsSync(dumpFileDynamic)) + done() + }) + }) +}) diff --git a/spec/api-net-spec.js b/spec/api-net-spec.js index 4f9abfe1a5f..2beacb67c25 100644 --- a/spec/api-net-spec.js +++ b/spec/api-net-spec.js @@ -1524,7 +1524,7 @@ describe('net module', () => { }) }) - describe('Stability and performance', (done) => { + describe('Stability and performance', () => { it('should free unreferenced, never-started request objects without crash', (done) => { const requestUrl = '/requestUrl' ipcRenderer.once('api-net-spec-done', () => { diff --git a/spec/fixtures/api/net-log/main.js b/spec/fixtures/api/net-log/main.js new file mode 100644 index 00000000000..54fa53965cd --- /dev/null +++ b/spec/fixtures/api/net-log/main.js @@ -0,0 +1,33 @@ +const {app, net, netLog} = require('electron') + +function request () { + return new Promise((resolve) => { + const req = net.request(process.env.TEST_REQUEST_URL) + req.on('response', () => { + resolve() + }) + req.end() + }) +} + +function stopLogging () { + return new Promise((resolve) => { + netLog.stopLogging(() => { + resolve() + }) + }) +} + +app.on('ready', async () => { + if (process.env.TEST_DUMP_FILE) { + netLog.startLogging(process.env.TEST_DUMP_FILE) + } + + await request() + + if (process.env.TEST_MANUAL_STOP) { + await stopLogging() + } + + app.quit() +}) diff --git a/spec/fixtures/api/net-log/package.json b/spec/fixtures/api/net-log/package.json new file mode 100644 index 00000000000..59272007963 --- /dev/null +++ b/spec/fixtures/api/net-log/package.json @@ -0,0 +1,4 @@ +{ + "name": "net-log", + "main": "main.js" +}