Skip to content

Commit

Permalink
src: add cli option to preserve env vars on dr
Browse files Browse the repository at this point in the history
  • Loading branch information
RafaelGSS committed Nov 2, 2024
1 parent 5ce3d10 commit ac4015d
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 3 deletions.
9 changes: 9 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,15 @@ Enables report to be generated upon receiving the specified (or predefined)
signal to the running Node.js process. The signal to trigger the report is
specified through `--report-signal`.

### `--report-preserve-env`

<!-- YAML
added: REPLACEME
-->

When `--report-preserve-env` is passed the diagnostic report generated will not
contain the `environmentVariables` data.

### `--report-signal=signal`

<!-- YAML
Expand Down
10 changes: 10 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -3494,6 +3494,16 @@ const { report } = require('node:process');
console.log(`Report on exception: ${report.reportOnUncaughtException}`);
```
### `process.report.preserveEnv`
<!-- YAML
added: REPLACEME
-->
* {boolean}
If `true`, a diagnostic report is generated without the environment variables.
### `process.report.signal`
<!-- YAML
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/process/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ const report = {

nr.setReportOnUncaughtException(trigger);
},
get preserveEnv() {
return !nr.shouldPreserveEnvironmentVariables();
},
};

function addSignalHandler(sig) {
Expand Down
4 changes: 4 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,10 @@ inline void Environment::RemoveHeapSnapshotNearHeapLimitCallback(
heap_limit);
}

inline bool Environment::ShouldPreserveEnvOnReport() const {
return options_->report_preserve_env;
}

inline void Environment::SetAsyncResourceContextFrame(
std::uintptr_t async_resource_handle,
v8::Global<v8::Value>&& context_frame) {
Expand Down
2 changes: 2 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,8 @@ class Environment final : public MemoryRetainer {

inline void RemoveHeapSnapshotNearHeapLimitCallback(size_t heap_limit);

inline bool ShouldPreserveEnvOnReport() const;

// Field identifiers for exit_info_
enum ExitInfoField {
kExiting = 0,
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
" (default: current working directory)",
&EnvironmentOptions::diagnostic_dir,
kAllowedInEnvvar);
AddOption("--report-preserve-env",
"Preserve environment variables when generating report",
&EnvironmentOptions::report_preserve_env,
kAllowedInEnvvar);
AddOption("--dns-result-order",
"set default value of verbatim in dns.lookup. Options are "
"'ipv4first' (IPv4 addresses are placed before IPv6 addresses) "
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ class EnvironmentOptions : public Options {
#endif // HAVE_INSPECTOR
std::string redirect_warnings;
std::string diagnostic_dir;
bool report_preserve_env = false;
std::string env_file;
std::string optional_env_file;
bool has_env_file_string = false;
Expand Down
10 changes: 8 additions & 2 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ static void PrintJavaScriptErrorProperties(JSONWriter* writer,
static void PrintNativeStack(JSONWriter* writer);
static void PrintResourceUsage(JSONWriter* writer);
static void PrintGCStatistics(JSONWriter* writer, Isolate* isolate);
static void PrintEnvironmentVariables(JSONWriter* writer);
static void PrintSystemInformation(JSONWriter* writer);
static void PrintLoadedLibraries(JSONWriter* writer);
static void PrintComponentVersions(JSONWriter* writer);
Expand Down Expand Up @@ -249,6 +250,9 @@ static void WriteNodeReport(Isolate* isolate,
writer.json_arrayend();

// Report operating system information
if (env->ShouldPreserveEnvOnReport()) {
PrintEnvironmentVariables(&writer);
}
PrintSystemInformation(&writer);

writer.json_objectend();
Expand Down Expand Up @@ -694,8 +698,7 @@ static void PrintResourceUsage(JSONWriter* writer) {
#endif // RUSAGE_THREAD
}

// Report operating system information.
static void PrintSystemInformation(JSONWriter* writer) {
static void PrintEnvironmentVariables(JSONWriter* writer) {
uv_env_item_t* envitems;
int envcount;
int r;
Expand All @@ -715,7 +718,10 @@ static void PrintSystemInformation(JSONWriter* writer) {
}

writer->json_objectend();
}

// Report operating system information.
static void PrintSystemInformation(JSONWriter* writer) {
#ifndef _WIN32
static struct {
const char* description;
Expand Down
11 changes: 11 additions & 0 deletions src/node_report_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ static void ShouldReportOnUncaughtException(
env->isolate_data()->options()->report_uncaught_exception);
}

static void ShouldPreserveEnvironmentVariables(
const FunctionCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
info.GetReturnValue().Set(env->ShouldPreserveEnvOnReport());
}

static void SetReportOnUncaughtException(
const FunctionCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
Expand Down Expand Up @@ -202,6 +208,10 @@ static void Initialize(Local<Object> exports,
exports,
"shouldReportOnUncaughtException",
ShouldReportOnUncaughtException);
SetMethod(context,
exports,
"shouldPreserveEnvironmentVariables",
ShouldPreserveEnvironmentVariables);
SetMethod(context,
exports,
"setReportOnUncaughtException",
Expand All @@ -226,6 +236,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(ShouldReportOnSignal);
registry->Register(SetReportOnSignal);
registry->Register(ShouldReportOnUncaughtException);
registry->Register(ShouldPreserveEnvironmentVariables);
registry->Register(SetReportOnUncaughtException);
}

Expand Down
6 changes: 5 additions & 1 deletion test/common/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ function _validateContent(report, fields = []) {

// Verify that all sections are present as own properties of the report.
const sections = ['header', 'nativeStack', 'javascriptStack', 'libuv',
'environmentVariables', 'sharedObjects', 'resourceUsage', 'workers'];
'sharedObjects', 'resourceUsage', 'workers'];

if (!process.report.preserveEnv)
sections.push('environmentVariables');

if (!isWindows)
sections.push('userLimits');

Expand Down
80 changes: 80 additions & 0 deletions test/report/test-report-writereport-preserve-env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Flags: --report-preserve-env
'use strict';

// Test producing a report via API call, using the no-hooks/no-signal interface.
require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const helper = require('../common/report');
const tmpdir = require('../common/tmpdir');

tmpdir.refresh();
process.report.directory = tmpdir.path;

function validate() {
const reports = helper.findReports(process.pid, tmpdir.path);
assert.strictEqual(reports.length, 1);
helper.validate(reports[0], arguments[0]);
fs.unlinkSync(reports[0]);
return reports[0];
}

{
// Test with no arguments.
process.report.writeReport();
validate();
}

{
// Test with an error argument.
process.report.writeReport(new Error('test error'));
validate();
}

{
// Test with an error with one line stack
const error = new Error();
error.stack = 'only one line';
process.report.writeReport(error);
validate();
}

{
const error = new Error();
error.foo = 'goo';
process.report.writeReport(error);
validate([['javascriptStack.errorProperties.foo', 'goo']]);
}

{
// Test with a file argument.
const file = process.report.writeReport('custom-name-1.json');
const absolutePath = tmpdir.resolve(file);
assert.strictEqual(helper.findReports(process.pid, tmpdir.path).length, 0);
assert.strictEqual(file, 'custom-name-1.json');
helper.validate(absolutePath);
fs.unlinkSync(absolutePath);
}

{
// Test with file and error arguments.
const file = process.report.writeReport('custom-name-2.json',
new Error('test error'));
const absolutePath = tmpdir.resolve(file);
assert.strictEqual(helper.findReports(process.pid, tmpdir.path).length, 0);
assert.strictEqual(file, 'custom-name-2.json');
helper.validate(absolutePath);
fs.unlinkSync(absolutePath);
}

{
// Test with a filename option.
process.report.filename = 'custom-name-3.json';
const file = process.report.writeReport();
assert.strictEqual(helper.findReports(process.pid, tmpdir.path).length, 0);
const filename = path.join(process.report.directory, 'custom-name-3.json');
assert.strictEqual(file, process.report.filename);
helper.validate(filename);
fs.unlinkSync(filename);
}

0 comments on commit ac4015d

Please sign in to comment.