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
facebook · Oct 9, 2019 · a483f80 · a483f80
1 parent 6140398
commit a483f80
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 72 deletions.
diff --git 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
@@ -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
@@ -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(/^.+? \((?=\/)/, '');
-}