Skip to content

Commit

Permalink
chore: improve contents.takeHeapSnapshot error messages (#37460)
Browse files Browse the repository at this point in the history
* chore: improve contents.takeHeapSnapshot error messages

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* fix wstring conversion issue

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
  • Loading branch information
trop[bot] and codebytere committed Mar 1, 2023
1 parent d6a6ef2 commit 587594e
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
16 changes: 12 additions & 4 deletions shell/browser/api/electron_api_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3517,18 +3517,26 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
base::File file(file_path,
base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
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;
}

Expand All @@ -3548,7 +3556,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)));
Expand Down
20 changes: 18 additions & 2 deletions spec/api-web-contents-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});

Expand Down

0 comments on commit 587594e

Please sign in to comment.