From 9b7958c2f28087f90d7e7635a20cc90ddc7c3a07 Mon Sep 17 00:00:00 2001
From: Riley Dulin <dulinr@fb.com>
Date: Wed, 12 Feb 2020 17:09:37 -0800
Subject: [PATCH] Have heap snapshots throw std::system_error instead of return
 a bool

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
---
 ReactCommon/jsi/jsi/decorator.h       |  8 ++++----
 ReactCommon/jsi/jsi/instrumentation.h |  8 ++------
 ReactCommon/jsi/jsi/jsi.cpp           | 10 ++++++----
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/ReactCommon/jsi/jsi/decorator.h b/ReactCommon/jsi/jsi/decorator.h
index 9616d19c7e5..1a2950d01d2 100644
--- a/ReactCommon/jsi/jsi/decorator.h
+++ b/ReactCommon/jsi/jsi/decorator.h
@@ -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(
diff --git a/ReactCommon/jsi/jsi/instrumentation.h b/ReactCommon/jsi/jsi/instrumentation.h
index 3de9bce1c79..f22a2094eaf 100644
--- a/ReactCommon/jsi/jsi/instrumentation.h
+++ b/ReactCommon/jsi/jsi/instrumentation.h
@@ -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(
diff --git a/ReactCommon/jsi/jsi/jsi.cpp b/ReactCommon/jsi/jsi/jsi.cpp
index e7b6ac94ed0..1fe8bb2e0d6 100644
--- a/ReactCommon/jsi/jsi/jsi.cpp
+++ b/ReactCommon/jsi/jsi/jsi.cpp
@@ -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 {