fix,v8: make v8.writeHeapSnapshot throw if fopen/fclose fail

doc/api/v8.md

@@ -267,6 +267,10 @@ disk unless [`v8.stopCoverage()`][] is invoked before the process exits.
 
 <!-- YAML
 added: v11.13.0
+changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/41373
+ description: An exception will now be thrown if the file could not be written.
 -->
 
 * `filename` {string} The file path where the V8 heap snapshot is to be

src/env.cc

@@ -1643,7 +1643,7 @@ size_t Environment::NearHeapLimitCallback(void* data,
  env->isolate()->RemoveNearHeapLimitCallback(NearHeapLimitCallback,
  initial_heap_limit);
 
- heap::WriteSnapshot(env->isolate(), filename.c_str());
+ heap::WriteSnapshot(env, filename.c_str());
  env->heap_limit_snapshot_taken_ += 1;
 
  // Don't take more snapshots than the number specified by

src/heap_utils.cc

@@ -308,21 +308,26 @@ class HeapSnapshotStream : public AsyncWrap,
  HeapSnapshotPointer snapshot_;
 };
 
-inline void TakeSnapshot(Isolate* isolate, v8::OutputStream* out) {
+inline void TakeSnapshot(Environment* env, v8::OutputStream* out) {
  HeapSnapshotPointer snapshot {
- isolate->GetHeapProfiler()->TakeHeapSnapshot() };
+ env->isolate()->GetHeapProfiler()->TakeHeapSnapshot() };
  snapshot->Serialize(out, HeapSnapshot::kJSON);
 }
 
 } // namespace
 
-bool WriteSnapshot(Isolate* isolate, const char* filename) {
+bool WriteSnapshot(Environment* env, const char* filename) {
  FILE* fp = fopen(filename, "w");
- if (fp == nullptr)
+ if (fp == nullptr) {
+ env->ThrowErrnoException(errno, "open");
  return false;
+ }
  FileOutputStream stream(fp);
- TakeSnapshot(isolate, &stream);
- fclose(fp);
+ TakeSnapshot(env, &stream);
+ if (fclose(fp) == EOF) {
+ env->ThrowErrnoException(errno, "close");
+ return false;
+ }
  return true;
 }
 
@@ -374,7 +379,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
 
  if (filename_v->IsUndefined()) {
  DiagnosticFilename name(env, "Heap", "heapsnapshot");
- if (!WriteSnapshot(isolate, *name))
+ if (!WriteSnapshot(env, *name))
  return;
  if (String::NewFromUtf8(isolate, *name).ToLocal(&filename_v)) {
  args.GetReturnValue().Set(filename_v);
@@ -384,7 +389,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
 
  BufferValue path(isolate, filename_v);
  CHECK_NOT_NULL(*path);
- if (!WriteSnapshot(isolate, *path))
+ if (!WriteSnapshot(env, *path))
  return;
  return args.GetReturnValue().Set(filename_v);
 }

src/node_internals.h

@@ -379,7 +379,7 @@ class DiagnosticFilename {
 };
 
 namespace heap {
-bool WriteSnapshot(v8::Isolate* isolate, const char* filename);
+bool WriteSnapshot(Environment* env, const char* filename);
 }
 
 class TraceEventScope {

test/sequential/test-heapdump.js

@@ -24,6 +24,19 @@ process.chdir(tmpdir.path);
  fs.accessSync(heapdump);
 }
 
+{
+ const readonlyFile = 'ro';
+ fs.writeFileSync(readonlyFile, Buffer.alloc(0), { mode: 0o444 });
+ assert.throws(() => {
+ writeHeapSnapshot(readonlyFile);
+ }, (e) => {
+ assert.ok(e, 'writeHeapSnapshot should error');
+ assert.strictEqual(e.code, 'EACCES');
+ assert.strictEqual(e.syscall, 'open');
+ return true;
+ });
+}
+
 [1, true, {}, [], null, Infinity, NaN].forEach((i) => {
  assert.throws(() => writeHeapSnapshot(i), {
  code: 'ERR_INVALID_ARG_TYPE',