diff --git a/src/node.h b/src/node.h index e401299f60315e..d6f6509f886959 100644 --- a/src/node.h +++ b/src/node.h @@ -75,8 +75,9 @@ #include "v8-platform.h" // NOLINT(build/include_order) #include "node_version.h" // NODE_MODULE_VERSION -#include #include +#include +#include // We cannot use __POSIX__ in this header because that's only defined when // building Node.js. @@ -620,16 +621,25 @@ NODE_EXTERN v8::MaybeLocal PrepareStackTraceCallback( // Writes a diagnostic report to a file. If filename is not provided, the // default filename includes the date, time, PID, and a sequence number. // The report's JavaScript stack trace is taken from err, if present. -// If isolate or env is nullptr, no information about the isolate and env +// If isolate is nullptr, no information about the JavaScript environment // is included in the report. +// Returns the filename of the written report. NODE_EXTERN std::string TriggerNodeReport(v8::Isolate* isolate, - Environment* env, + const char* message, + const char* trigger, + const std::string& filename, + v8::Local error); +NODE_EXTERN std::string TriggerNodeReport(Environment* env, const char* message, const char* trigger, const std::string& filename, v8::Local error); NODE_EXTERN void GetNodeReport(v8::Isolate* isolate, - Environment* env, + const char* message, + const char* trigger, + v8::Local error, + std::ostream& out); +NODE_EXTERN void GetNodeReport(Environment* env, const char* message, const char* trigger, v8::Local error, diff --git a/src/node_errors.cc b/src/node_errors.cc index 9a8a55f689528a..8f5950492e795a 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -474,10 +474,6 @@ void OnFatalError(const char* location, const char* message) { } Isolate* isolate = Isolate::TryGetCurrent(); - Environment* env = nullptr; - if (isolate != nullptr) { - env = Environment::GetCurrent(isolate); - } bool report_on_fatalerror; { Mutex::ScopedLock lock(node::per_process::cli_options_mutex); @@ -485,7 +481,7 @@ void OnFatalError(const char* location, const char* message) { } if (report_on_fatalerror) { - TriggerNodeReport(isolate, env, message, "FatalError", "", Local()); + TriggerNodeReport(isolate, message, "FatalError", "", Local()); } fflush(stderr); @@ -503,10 +499,6 @@ void OOMErrorHandler(const char* location, bool is_heap_oom) { } Isolate* isolate = Isolate::TryGetCurrent(); - Environment* env = nullptr; - if (isolate != nullptr) { - env = Environment::GetCurrent(isolate); - } bool report_on_fatalerror; { Mutex::ScopedLock lock(node::per_process::cli_options_mutex); @@ -514,7 +506,7 @@ void OOMErrorHandler(const char* location, bool is_heap_oom) { } if (report_on_fatalerror) { - TriggerNodeReport(isolate, env, message, "OOMError", "", Local()); + TriggerNodeReport(isolate, message, "OOMError", "", Local()); } fflush(stderr); diff --git a/src/node_report.cc b/src/node_report.cc index 8c42d192cf1017..3970f4ec53127e 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -222,12 +222,8 @@ static void WriteNodeReport(Isolate* isolate, expected_results += w->RequestInterrupt([&](Environment* env) { std::ostringstream os; - GetNodeReport(env->isolate(), - env, - "Worker thread subreport", - trigger, - Local(), - os); + GetNodeReport( + env, "Worker thread subreport", trigger, Local(), os); Mutex::ScopedLock lock(workers_mutex); worker_infos.emplace_back(os.str()); @@ -790,7 +786,19 @@ static void PrintRelease(JSONWriter* writer) { // External function to trigger a report, writing to file. std::string TriggerNodeReport(Isolate* isolate, - Environment* env, + const char* message, + const char* trigger, + const std::string& name, + Local error) { + Environment* env = nullptr; + if (isolate != nullptr) { + env = Environment::GetCurrent(isolate); + } + return TriggerNodeReport(env, message, trigger, name, error); +} + +// External function to trigger a report, writing to file. +std::string TriggerNodeReport(Environment* env, const char* message, const char* trigger, const std::string& name, @@ -859,6 +867,11 @@ std::string TriggerNodeReport(Isolate* isolate, Mutex::ScopedLock lock(per_process::cli_options_mutex); compact = per_process::cli_options->report_compact; } + + Isolate* isolate = nullptr; + if (env != nullptr) { + isolate = env->isolate(); + } report::WriteNodeReport( isolate, env, message, trigger, filename, *outstream, error, compact); @@ -876,11 +889,28 @@ std::string TriggerNodeReport(Isolate* isolate, // External function to trigger a report, writing to a supplied stream. void GetNodeReport(Isolate* isolate, - Environment* env, const char* message, const char* trigger, Local error, std::ostream& out) { + Environment* env = nullptr; + if (isolate != nullptr) { + env = Environment::GetCurrent(isolate); + } + report::WriteNodeReport( + isolate, env, message, trigger, "", out, error, false); +} + +// External function to trigger a report, writing to a supplied stream. +void GetNodeReport(Environment* env, + const char* message, + const char* trigger, + Local error, + std::ostream& out) { + Isolate* isolate = nullptr; + if (env != nullptr) { + isolate = env->isolate(); + } report::WriteNodeReport( isolate, env, message, trigger, "", out, error, false); } diff --git a/src/node_report.h b/src/node_report.h index 3cd071afbc0285..7a2e817ac82f6b 100644 --- a/src/node_report.h +++ b/src/node_report.h @@ -35,21 +35,6 @@ void WriteReport(const v8::FunctionCallbackInfo& info); void GetReport(const v8::FunctionCallbackInfo& info); } // namespace report - -// Function declarations - functions in src/node_report.cc -std::string TriggerNodeReport(v8::Isolate* isolate, - Environment* env, - const char* message, - const char* trigger, - const std::string& name, - v8::Local error); -void GetNodeReport(v8::Isolate* isolate, - Environment* env, - const char* message, - const char* trigger, - v8::Local error, - std::ostream& out); - } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/node_report_module.cc b/src/node_report_module.cc index 29da71b9e2a71f..884f9ec325979a 100644 --- a/src/node_report_module.cc +++ b/src/node_report_module.cc @@ -44,8 +44,7 @@ void WriteReport(const FunctionCallbackInfo& info) { else error = Local(); - filename = TriggerNodeReport( - isolate, env, *message, *trigger, filename, error); + filename = TriggerNodeReport(env, *message, *trigger, filename, error); // Return value is the report filename info.GetReturnValue().Set( String::NewFromUtf8(isolate, filename.c_str()).ToLocalChecked()); @@ -65,8 +64,7 @@ void GetReport(const FunctionCallbackInfo& info) { else error = Local(); - GetNodeReport( - isolate, env, "JavaScript API", __func__, error, out); + GetNodeReport(env, "JavaScript API", __func__, error, out); // Return value is the contents of a report as a string. info.GetReturnValue().Set( diff --git a/test/addons/report-api/binding.cc b/test/addons/report-api/binding.cc index 476b01512151b2..f52da3c765d7df 100644 --- a/test/addons/report-api/binding.cc +++ b/test/addons/report-api/binding.cc @@ -11,7 +11,21 @@ void TriggerReport(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); node::TriggerNodeReport( - isolate, + isolate, "FooMessage", "BarTrigger", std::string(), Local()); +} + +void TriggerReportNoIsolate(const FunctionCallbackInfo& args) { + node::TriggerNodeReport(static_cast(nullptr), + "FooMessage", + "BarTrigger", + std::string(), + Local()); +} + +void TriggerReportEnv(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + + node::TriggerNodeReport( node::GetCurrentEnvironment(isolate->GetCurrentContext()), "FooMessage", "BarTrigger", @@ -19,8 +33,21 @@ void TriggerReport(const FunctionCallbackInfo& args) { Local()); } +void TriggerReportNoEnv(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + + node::TriggerNodeReport(static_cast(nullptr), + "FooMessage", + "BarTrigger", + std::string(), + Local()); +} + void init(Local exports) { NODE_SET_METHOD(exports, "triggerReport", TriggerReport); + NODE_SET_METHOD(exports, "triggerReportNoIsolate", TriggerReportNoIsolate); + NODE_SET_METHOD(exports, "triggerReportEnv", TriggerReportEnv); + NODE_SET_METHOD(exports, "triggerReportNoEnv", TriggerReportNoEnv); } NODE_MODULE(NODE_GYP_MODULE_NAME, init) diff --git a/test/addons/report-api/test.js b/test/addons/report-api/test.js index 8bd08beb8bfeda..e5000f56a584e1 100644 --- a/test/addons/report-api/test.js +++ b/test/addons/report-api/test.js @@ -9,11 +9,11 @@ const tmpdir = require('../../common/tmpdir'); const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`); const addon = require(binding); -function myAddonMain() { +function myAddonMain(method, hasJavaScriptFrames) { tmpdir.refresh(); process.report.directory = tmpdir.path; - addon.triggerReport(); + addon[method](); const reports = helper.findReports(process.pid, tmpdir.path); assert.strictEqual(reports.length, 1); @@ -26,7 +26,19 @@ function myAddonMain() { assert.strictEqual(content.header.trigger, 'BarTrigger'); // Check that the javascript stack is present. - assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('myAddonMain')), 0); + if (hasJavaScriptFrames) { + assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('myAddonMain')), 0); + } else { + assert.strictEqual(content.javascriptStack, undefined); + } } -myAddonMain(); +const methods = [ + ['triggerReport', true], + ['triggerReportNoIsolate', false], + ['triggerReportEnv', true], + ['triggerReportNoEnv', false], +]; +for (const [method, hasJavaScriptFrames] of methods) { + myAddonMain(method, hasJavaScriptFrames); +} diff --git a/test/cctest/test_report.cc b/test/cctest/test_report.cc index ab76188a04f27f..861fa40385e206 100644 --- a/test/cctest/test_report.cc +++ b/test/cctest/test_report.cc @@ -25,12 +25,15 @@ class ReportTest : public EnvironmentTestFixture { } }; -TEST_F(ReportTest, ReportWithNoIsolateAndEnv) { +TEST_F(ReportTest, ReportWithNoIsolate) { SealHandleScope handle_scope(isolate_); std::ostringstream oss; - node::GetNodeReport( - nullptr, nullptr, "FooMessage", "BarTrigger", Local(), oss); + node::GetNodeReport(static_cast(nullptr), + "FooMessage", + "BarTrigger", + Local(), + oss); // Simple checks on the output string contains the message and trigger. std::string actual = oss.str(); @@ -38,7 +41,54 @@ TEST_F(ReportTest, ReportWithNoIsolateAndEnv) { EXPECT_NE(actual.find("BarTrigger"), std::string::npos); } -TEST_F(ReportTest, ReportWithIsolateAndEnv) { +TEST_F(ReportTest, ReportWithNoEnv) { + SealHandleScope handle_scope(isolate_); + + std::ostringstream oss; + node::GetNodeReport(static_cast(nullptr), + "FooMessage", + "BarTrigger", + Local(), + oss); + + // Simple checks on the output string contains the message and trigger. + std::string actual = oss.str(); + EXPECT_NE(actual.find("FooMessage"), std::string::npos); + EXPECT_NE(actual.find("BarTrigger"), std::string::npos); +} + +TEST_F(ReportTest, ReportWithIsolate) { + const HandleScope handle_scope(isolate_); + const Argv argv; + Env env{handle_scope, argv}; + + Local context = isolate_->GetCurrentContext(); + Local fn = + Function::New(context, [](const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + HandleScope scope(isolate); + + std::ostringstream oss; + node::GetNodeReport(isolate, "FooMessage", "BarTrigger", args[0], oss); + + // Simple checks on the output string contains the message and trigger. + std::string actual = oss.str(); + EXPECT_NE(actual.find("FooMessage"), std::string::npos); + EXPECT_NE(actual.find("BarTrigger"), std::string::npos); + + report_callback_called = true; + }).ToLocalChecked(); + + context->Global() + ->Set(context, String::NewFromUtf8(isolate_, "foo").ToLocalChecked(), fn) + .FromJust(); + + node::LoadEnvironment(*env, "foo()").ToLocalChecked(); + + EXPECT_TRUE(report_callback_called); +} + +TEST_F(ReportTest, ReportWithEnv) { const HandleScope handle_scope(isolate_); const Argv argv; Env env{handle_scope, argv}; @@ -48,12 +98,14 @@ TEST_F(ReportTest, ReportWithIsolateAndEnv) { Function::New(context, [](const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); HandleScope scope(isolate); - Environment* env = - node::GetCurrentEnvironment(isolate->GetCurrentContext()); std::ostringstream oss; node::GetNodeReport( - isolate, env, "FooMessage", "BarTrigger", args[0], oss); + node::GetCurrentEnvironment(isolate->GetCurrentContext()), + "FooMessage", + "BarTrigger", + args[0], + oss); // Simple checks on the output string contains the message and trigger. std::string actual = oss.str();