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

@@ -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<v8::Value> 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<v8::FunctionTemplate> 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<v8::Object> exports,
+                v8::Local<v8::Value> unused,
+                v8::Local<v8::Context> 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)

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 <string>
+#include "brightray/browser/net_log.h"
+#include "native_mate/wrappable.h"
+
+namespace atom {
+
+namespace api {
+
+class NetLog : public mate::Wrappable<NetLog> {
+ public:
+  static v8::Local<v8::Value> Create(v8::Isolate* isolate);
+
+  static void BuildPrototype(v8::Isolate* isolate,
+                             v8::Local<v8::FunctionTemplate> 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_

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)                   \

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<void(bool, bool)>& callback) {
@@ -107,7 +106,7 @@ void BrowserClient::GetAdditionalWebUISchemes(
   additional_schemes->push_back(content::kChromeDevToolsScheme);
 }
 
-net::NetLog* BrowserClient::GetNetLog() {
+NetLog* BrowserClient::GetNetLog() {
   return &net_log_;
 }
 

brightray/browser/browser_client.h

@@ -45,7 +45,7 @@ class BrowserClient : public content::ContentBrowserClient {
       std::vector<std::string>* additional_schemes) override;
   void GetAdditionalWebUISchemes(
       std::vector<std::string>* additional_schemes) override;
-  net::NetLog* GetNetLog() override;
+  NetLog* GetNetLog() override;
   base::FilePath GetDefaultDownloadDirectory() override;
   content::DevToolsManagerDelegate* GetDevToolsManagerDelegate() override;
   std::string GetApplicationLocale() override;

brightray/browser/net_log.cc

@@ -6,7 +6,6 @@
 
 #include <utility>
 
-#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<base::DictionaryValue> 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<base::Value> constants(GetConstants());
+  base::FilePath log_path;
+  log_path = command_line->GetSwitchValuePath(switches::kLogNetLog);
+  if (log_path.empty())
+    return;
+
+  std::unique_ptr<base::Value> 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<base::Value> 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

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<net::FileNetLogObserver> file_net_log_observer_;
+  std::unique_ptr<net::FileNetLogObserver> dynamic_file_net_log_observer_;
+  base::FilePath dynamic_file_net_log_path_;
 
   DISALLOW_COPY_AND_ASSIGN(NetLog);
 };

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)

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.

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',

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'},

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

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()
+    })
+  })
+})

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', () => {

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()
+})

spec/fixtures/api/net-log/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "net-log",
+  "main": "main.js"
+}