From a483f802fddfd927f2baa0d95e2b4094d452cddd Mon Sep 17 00:00:00 2001
From: Moti Zilberman <moti@fb.com>
Date: Wed, 9 Oct 2019 05:46:34 -0700
Subject: [PATCH] Remove framesToPop from bridge

Summary: Removes the use of `framesToPop` from method wrappers in the RN bridge.

Reviewed By: rubennorte

Differential Revision: D17764986

fbshipit-source-id: f2fac0c33a9a7c821bb920fa65b92a4672150a38
---
 Libraries/BatchedBridge/MessageQueue.js       | 14 +----
 Libraries/BatchedBridge/NativeModules.js      |  5 +-
 .../__tests__/NativeModules-test.js           | 56 -------------------
 3 files changed, 3 insertions(+), 72 deletions(-)

diff --git a/Libraries/BatchedBridge/MessageQueue.js b/Libraries/BatchedBridge/MessageQueue.js
index 93ac7436c19..37d35b659b7 100644
--- a/Libraries/BatchedBridge/MessageQueue.js
+++ b/Libraries/BatchedBridge/MessageQueue.js
@@ -190,19 +190,7 @@ class MessageQueue {
       );
     }
     this.processCallbacks(moduleID, methodID, params, onFail, onSucc);
-    try {
-      return global.nativeCallSyncHook(moduleID, methodID, params);
-    } catch (e) {
-      if (
-        typeof e === 'object' &&
-        e != null &&
-        typeof e.framesToPop === 'undefined' &&
-        /^Exception in HostFunction: /.test(e.message)
-      ) {
-        e.framesToPop = 2;
-      }
-      throw e;
-    }
+    return global.nativeCallSyncHook(moduleID, methodID, params);
   }
 
   processCallbacks(
diff --git a/Libraries/BatchedBridge/NativeModules.js b/Libraries/BatchedBridge/NativeModules.js
index 703d9dbe447..a0c5b074b59 100644
--- a/Libraries/BatchedBridge/NativeModules.js
+++ b/Libraries/BatchedBridge/NativeModules.js
@@ -94,10 +94,9 @@ function loadModule(name: string, moduleID: number): ?Object {
 function genMethod(moduleID: number, methodID: number, type: MethodType) {
   let fn = null;
   if (type === 'promise') {
-    fn = function(...args: Array<any>) {
+    fn = function promiseMethodWrapper(...args: Array<any>) {
       // In case we reject, capture a useful stack trace here.
       const enqueueingFrameError: ExtendedError = new Error();
-      enqueueingFrameError.framesToPop = 1;
       return new Promise((resolve, reject) => {
         BatchedBridge.enqueueNativeCall(
           moduleID,
@@ -110,7 +109,7 @@ function genMethod(moduleID: number, methodID: number, type: MethodType) {
       });
     };
   } else {
-    fn = function(...args: Array<any>) {
+    fn = function nonPromiseMethodWrapper(...args: Array<any>) {
       const lastArg = args.length > 0 ? args[args.length - 1] : null;
       const secondLastArg = args.length > 1 ? args[args.length - 2] : null;
       const hasSuccessCallback = typeof lastArg === 'function';
diff --git a/Libraries/BatchedBridge/__tests__/NativeModules-test.js b/Libraries/BatchedBridge/__tests__/NativeModules-test.js
index 498eeab8b7c..bedff6127d9 100644
--- a/Libraries/BatchedBridge/__tests__/NativeModules-test.js
+++ b/Libraries/BatchedBridge/__tests__/NativeModules-test.js
@@ -167,11 +167,6 @@ describe('MessageQueue', function() {
     }).toThrow();
     await expect(promise1).rejects.toBeInstanceOf(Error);
     await expect(promise1).rejects.toMatchObject({message: 'firstFailure'});
-    // Check that we get a useful stack trace from failures.
-    const error = await promise1.catch(x => x);
-    expect(getLineFromFrame(parseErrorStack(error)[0])).toContain(
-      'NativeModules.RemoteModule1.promiseReturningMethod(',
-    );
 
     // Handle the second remote invocation by signaling success.
     BatchedBridge.__invokeCallback(secondSuccCBID, ['secondSucc']);
@@ -211,36 +206,6 @@ describe('MessageQueue', function() {
       });
     });
 
-    it('throwing a "native" exception gets framesToPop = 2', function() {
-      global.nativeCallSyncHook = () => {
-        throw new Error('Exception in HostFunction: foo');
-      };
-      let error;
-      try {
-        NativeModules.RemoteModule1.syncMethod('paloAlto', 'menloPark');
-      } catch (e) {
-        error = e;
-      }
-      // We can't test this behaviour with `getLineFromFrame` because our mock
-      // function adds an extra frame, so check `framesToPop` directly instead.
-      expect(error.framesToPop).toBe(2);
-    });
-
-    it('throwing a "native" exception preserves framesToPop if set', function() {
-      global.nativeCallSyncHook = () => {
-        const e = new Error('Exception in HostFunction: foo');
-        e.framesToPop = 42;
-        throw e;
-      };
-      let error;
-      try {
-        NativeModules.RemoteModule1.syncMethod('paloAlto', 'menloPark');
-      } catch (e) {
-        error = e;
-      }
-      expect(error.framesToPop).toBe(42);
-    });
-
     it('returning a value', function() {
       global.nativeCallSyncHook = jest.fn(() => {
         return 'secondSucc';
@@ -259,24 +224,3 @@ describe('MessageQueue', function() {
     });
   });
 });
-
-const linesByFile = new Map();
-
-function getLineFromFrame({lineNumber /* 1-based */, file}) {
-  if (file == null) {
-    return null;
-  }
-  const cleanedFile = cleanFileName(file);
-  const lines =
-    linesByFile.get(cleanedFile) ||
-    fs.readFileSync(cleanedFile, 'utf8').split('\n');
-  if (!linesByFile.has(cleanedFile)) {
-    linesByFile.set(cleanedFile, lines);
-  }
-  return (lines[lineNumber - 1] || '').trim();
-}
-
-// Works around a parseErrorStack bug involving `new X` stack frames.
-function cleanFileName(file) {
-  return file.replace(/^.+? \((?=\/)/, '');
-}