Skip to content

Commit

Permalink
Have heap snapshots throw std::system_error instead of return a bool
Browse files Browse the repository at this point in the history
Summary:
Instead of returning a `bool` which gives no information about the cause of the error,
return `void` and throw when there's some error.

Another alternative is returning `std::error_code`, but that's less flexible than throwing, and
this API already supports throwing.

Changelog: [Internal]

Reviewed By: jbower-fb

Differential Revision: D19170033

fbshipit-source-id: 870cd996a1a53c94524455f31765c1da99f57a1d
  • Loading branch information
dulinriley authored and facebook-github-bot committed Feb 13, 2020
1 parent aa41fd5 commit 9b7958c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
8 changes: 4 additions & 4 deletions ReactCommon/jsi/jsi/decorator.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,12 +335,12 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
plain().instrumentation().collectGarbage();
}

bool createSnapshotToFile(const std::string& path) override {
return plain().instrumentation().createSnapshotToFile(path);
void createSnapshotToFile(const std::string& path) override {
plain().instrumentation().createSnapshotToFile(path);
}

bool createSnapshotToStream(std::ostream& os) override {
return plain().instrumentation().createSnapshotToStream(os);
void createSnapshotToStream(std::ostream& os) override {
plain().instrumentation().createSnapshotToStream(os);
}

void writeBridgeTrafficTraceToFile(
Expand Down
8 changes: 2 additions & 6 deletions ReactCommon/jsi/jsi/instrumentation.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,12 @@ class Instrumentation {
/// Captures the heap to a file
///
/// \param path to save the heap capture
///
/// \return true iff the heap capture succeeded
virtual bool createSnapshotToFile(const std::string& path) = 0;
virtual void createSnapshotToFile(const std::string& path) = 0;

/// Captures the heap to an output stream
///
/// \param os output stream to write to.
///
/// \return true iff the heap capture succeeded.
virtual bool createSnapshotToStream(std::ostream& os) = 0;
virtual void createSnapshotToStream(std::ostream& os) = 0;

/// Write a trace of bridge traffic to the given file name.
virtual void writeBridgeTrafficTraceToFile(
Expand Down
10 changes: 6 additions & 4 deletions ReactCommon/jsi/jsi/jsi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,14 @@ Instrumentation& Runtime::instrumentation() {

void collectGarbage() override {}

bool createSnapshotToFile(const std::string&) override {
return false;
void createSnapshotToFile(const std::string&) override {
throw JSINativeException(
"Default instrumentation cannot create a heap snapshot");
}

bool createSnapshotToStream(std::ostream&) override {
return false;
void createSnapshotToStream(std::ostream&) override {
throw JSINativeException(
"Default instrumentation cannot create a heap snapshot");
}

void writeBridgeTrafficTraceToFile(const std::string&) const override {
Expand Down

0 comments on commit 9b7958c

Please sign in to comment.