src,lib: support path in v8.writeHeapSnapshot output

Fixes: #58857
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>

Author: juanarbol

lib/v8.js

@@ -77,16 +77,22 @@ const { getOptionValue } = require('internal/options');
  * @param {string} [filename]
  * @param {{
  *   exposeInternals?: boolean,
- *   exposeNumericValues?: boolean
+ *   exposeNumericValues?: boolean,
+ *   path?: string,
  * }} [options]
  * @returns {string}
  */
 function writeHeapSnapshot(filename, options) {
   if (filename !== undefined) {
     filename = getValidatedPath(filename);
   }
+
+  if (options?.path !== undefined) {
+    options.path = getValidatedPath(options.path);
+  }
+
   const optionArray = getHeapSnapshotOptions(options);
-  return triggerHeapSnapshot(filename, optionArray);
+  return triggerHeapSnapshot(filename, optionArray, options?.path);
 }
 
 /**

src/heap_utils.cc

@@ -448,30 +448,58 @@ void CreateHeapSnapshotStream(const FunctionCallbackInfo<Value>& args) {
 void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = args.GetIsolate();
-  CHECK_EQ(args.Length(), 2);
+  CHECK_EQ(args.Length(), 3);
   Local<Value> filename_v = args[0];
   auto options = GetHeapSnapshotOptions(args[1]);
+  Local<String> path_v = args[2].As<String>();
+  bool has_path = !path_v->IsNullOrUndefined() && path_v->IsString();
+  std::string final_filename;
+
 
   if (filename_v->IsUndefined()) {
     DiagnosticFilename name(env, "Heap", "heapsnapshot");
-    THROW_IF_INSUFFICIENT_PERMISSIONS(
+    if (!has_path) {
+      THROW_IF_INSUFFICIENT_PERMISSIONS(
         env,
         permission::PermissionScope::kFileSystemWrite,
         Environment::GetCwd(env->exec_path()));
-    if (WriteSnapshot(env, *name, options).IsNothing()) return;
-    if (String::NewFromUtf8(isolate, *name).ToLocal(&filename_v)) {
-      args.GetReturnValue().Set(filename_v);
+      final_filename = *name;
+    } else {
+      BufferValue path(isolate, path_v);
+      CHECK_NOT_NULL(*path);
+      ToNamespacedPath(env, &path);
+      THROW_IF_INSUFFICIENT_PERMISSIONS(
+          env, permission::PermissionScope::kFileSystemWrite,
+          path.ToStringView());
+
+      final_filename = std::string(*path) + "/" +
+                       *name;  // NOLINT(readability/pointer_notation)
+    }
+  } else {
+    BufferValue filename(isolate, filename_v);
+    CHECK_NOT_NULL(*filename);
+
+    if (has_path) {
+      BufferValue path(isolate, path_v);
+      CHECK_NOT_NULL(*path);
+      ToNamespacedPath(env, &path);
+      THROW_IF_INSUFFICIENT_PERMISSIONS(
+          env, permission::PermissionScope::kFileSystemWrite,
+          std::string(*path) + "/" + std::string(*filename));
+      final_filename = std::string(*path) + "/" + std::string(*filename);
+    } else {
+      ToNamespacedPath(env, &filename);
+      THROW_IF_INSUFFICIENT_PERMISSIONS(
+          env, permission::PermissionScope::kFileSystemWrite,
+          filename.ToStringView());
+      final_filename = *filename;
     }
-    return;
   }
 
-  BufferValue path(isolate, filename_v);
-  CHECK_NOT_NULL(*path);
-  ToNamespacedPath(env, &path);
-  THROW_IF_INSUFFICIENT_PERMISSIONS(
-      env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
-  if (WriteSnapshot(env, *path, options).IsNothing()) return;
-  return args.GetReturnValue().Set(filename_v);
+  if (WriteSnapshot(env, final_filename.c_str(), options).IsNothing()) return;
+
+  args.GetReturnValue().Set(
+      String::NewFromUtf8(isolate, final_filename.c_str()).ToLocalChecked());
 }
 
 void Initialize(Local<Object> target,

test/common/heap.js

@@ -316,6 +316,18 @@ function getHeapSnapshotOptionTests() {
         ],
       }],
     },
+    // This is the same as the previous case, but with a path option.
+    // The path option will be mutated by the test
+    // test-write-heapsnapshot-options.js
+    // Refs: https://github.com/nodejs/node/issues/58857
+    {
+      options: { exposeNumericValues: true, path: 'TBD' },
+      expected: [{
+        children: [
+          { edge_name: 'numeric', node_name: 'smi number' },
+        ],
+      }],
+    },
   ];
   return {
     fixtures: fixtures.path('klass-with-fields.js'),

test/sequential/test-write-heapsnapshot-options.js

@@ -4,7 +4,9 @@
 
 require('../common');
 
+const assert = require('assert');
 const fs = require('fs');
+const tmpdir = require('../common/tmpdir');
 const { getHeapSnapshotOptionTests, recordState } = require('../common/heap');
 
 class ReadStream {
@@ -20,7 +22,12 @@ if (process.argv[2] === 'child') {
   const { writeHeapSnapshot } = require('v8');
   require(tests.fixtures);
   const { options, expected } = tests.cases[parseInt(process.argv[3])];
+  const { path } = options;
+  // If path is set, writeHeapSnapshot will write to the tmpdir.
+  if (path) options.path = tmpdir.path;
   const filename = writeHeapSnapshot(undefined, options);
+  // If path is set, the filename will be the provided path.
+  if (path) assert(filename.includes(tmpdir.path));
   const snapshot = recordState(new ReadStream(filename));
   console.log('Snapshot nodes', snapshot.snapshot.length);
   console.log('Searching for', expected[0].children);
@@ -30,8 +37,6 @@ if (process.argv[2] === 'child') {
 }
 
 const { spawnSync } = require('child_process');
-const assert = require('assert');
-const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
 
 // Start child processes to prevent the heap from growing too big.