Browse Source

chore: improve `contents.takeHeapSnapshot` error messages (#37461)

* chore: improve contents.takeHeapSnapshot error messages

Co-authored-by: Shelley Vohr <[email protected]>

* fix wstring conversion issue

Co-authored-by: Shelley Vohr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 2 years ago
parent
commit
03285c8faf
2 changed files with 30 additions and 6 deletions
  1. 12 4
      shell/browser/api/electron_api_web_contents.cc
  2. 18 2
      spec/api-web-contents-spec.ts

+ 12 - 4
shell/browser/api/electron_api_web_contents.cc

@@ -3512,18 +3512,26 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
   flags = base::File::AddFlagsForPassingToUntrustedProcess(flags);
   base::File file(file_path, flags);
   if (!file.IsValid()) {
-    promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+    promise.RejectWithErrorMessage(
+        "Failed to take heap snapshot with invalid file path " +
+#if BUILDFLAG(IS_WIN)
+        base::WideToUTF8(file_path.value()));
+#else
+        file_path.value());
+#endif
     return handle;
   }
 
   auto* frame_host = web_contents()->GetPrimaryMainFrame();
   if (!frame_host) {
-    promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+    promise.RejectWithErrorMessage(
+        "Failed to take heap snapshot with invalid webContents main frame");
     return handle;
   }
 
   if (!frame_host->IsRenderFrameLive()) {
-    promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+    promise.RejectWithErrorMessage(
+        "Failed to take heap snapshot with nonexistent render frame");
     return handle;
   }
 
@@ -3543,7 +3551,7 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
             if (success) {
               promise.Resolve();
             } else {
-              promise.RejectWithErrorMessage("takeHeapSnapshot failed");
+              promise.RejectWithErrorMessage("Failed to take heap snapshot");
             }
           },
           base::Owned(std::move(electron_renderer)), std::move(promise)));

+ 18 - 2
spec/api-web-contents-spec.ts

@@ -1812,8 +1812,24 @@ describe('webContents module', () => {
 
       await w.loadURL('about:blank');
 
-      const promise = w.webContents.takeHeapSnapshot('');
-      return expect(promise).to.be.eventually.rejectedWith(Error, 'takeHeapSnapshot failed');
+      const badPath = path.join('i', 'am', 'a', 'super', 'bad', 'path');
+      const promise = w.webContents.takeHeapSnapshot(badPath);
+      return expect(promise).to.be.eventually.rejectedWith(Error, `Failed to take heap snapshot with invalid file path ${badPath}`);
+    });
+
+    it('fails with invalid render process', async () => {
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          sandbox: true
+        }
+      });
+
+      const filePath = path.join(app.getPath('temp'), 'test.heapsnapshot');
+
+      w.webContents.destroy();
+      const promise = w.webContents.takeHeapSnapshot(filePath);
+      return expect(promise).to.be.eventually.rejectedWith(Error, 'Failed to take heap snapshot with nonexistent render frame');
     });
   });