src: add cli option to preserve env vars on dr #55697
Conversation
nodejs-github-bot
added
c++
| @@ -105,6 +105,9 @@ const report = { | |||
|
|
|||
| nr.setReportOnUncaughtException(trigger); | |||
| }, | |||
| get preserveEnv() { | |||
| return !nr.shouldPreserveEnvironmentVariables(); | |||
There was a problem hiding this comment.
Can this just be a constant exposed from C++ rather than a function?
There was a problem hiding this comment.
Also I think this should be cached, if we are keeping it as it is.
There was a problem hiding this comment.
We can cache it, yes, but at the moment this is only used inside our tests. That's why I just didn't care about it.
There was a problem hiding this comment.
I'll change it to a regular const. Possibly tomorrow.
| @@ -170,6 +170,12 @@ static void ShouldReportOnUncaughtException( | |||
| env->isolate_data()->options()->report_uncaught_exception); | |||
| } | |||
|
|
|||
| static void ShouldPreserveEnvironmentVariables( | |||
There was a problem hiding this comment.
Nit: a todo with a note of adding v8 fast api for this.
There was a problem hiding this comment.
I don't think there's a need for that now. See: #55697 (comment)
RafaelGSS
added
the
semver-minor
|
For consistency with |
This PR introduces a new cli flag
--report-preserve-envto avoid writing env vars on diagnostic reports.Some context: https://x.com/arthurfiorette/status/1848372069198614664