Promise.prototype.finally is not aligned with the most current spec draft

[MapTo0]

According to draft 4 of the Promise.prototype.finally spec , the callback parameter might not be callable.

Spec: https://tc39.es/proposal-promise-finally/
Similar issues: stefanpenner/es6-promise#336

[Jackman3005]

OMG. We have hundreds of Sentry issues that I've been debugging for weeks and weeks on and off, made so much worse by the really poor unhandled promise rejection stack I was getting in our react-native environment made this immensely difficult to track down. Turns out I was trusting the types too much and thought it was safe to do

myPromiseFn().finally()

Because the types say that the argument to finally is optional. I was doing this throughout my code because I was otherwise getting ESLint warnings saying I needed to await the promise that was returned. Doing this was my way of communicating that I know what I'm doing and do not intend to await this promise (in a useEffect() for example).

tl;dr

This library requires a function to be provided to finally and if not provided it throws with:

TypeError: undefined is not a function

Types Expectation

The types I find when clicking through to the definition of finally() are as follows:

interface Promise<T> {
    finally(onfinally?: (() => void) | undefined | null): Promise<T>
}

These types are provided directly from Typescript.

SEO for others in pain

Warning message I was receiving:

Possible Unhandled Promise Rejection (id: 0): TypeError: undefined is not a function

Sentry polyfills promise to make it nearly identical to the normal react-native promise, but also adds in sentry tracking of unhandled promise exceptions. This was the specific usage of this library I found.

Trying to find the original stack trace of unhandled promise rejections in this particular scenario was gruelingly difficult as the stack never included relevant code, just library code like the following:

(When Sentry was normally polyfilling)

TypeError: undefined is not a function
    at anonymous (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:125276:31)
    at tryCallOne (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:125101:16)
    at anonymous (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:125182:27)
    at apply (native)
    at anonymous (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26192:26)
    at _callTimer (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26111:17)
    at _callReactNativeMicrotasksPass (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26141:17)
    at callReactNativeMicrotasks (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26304:44)
    at __callReactNativeMicrotasks (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19761:46)
    at anonymous (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19573:45)
    at __guard (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19745:15)
    at flushedQueue (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19572:21)
    at callFunctionReturnFlushedQueue (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19557:33)

(When Sentry was set to enabled: false)

WARN  Possible Unhandled Promise Rejection (id: 5):
TypeError: undefined is not a function
TypeError: undefined is not a function
    at anonymous (/Users/distiller/react-native/sdks/hermes/build_iphonesimulator/lib/InternalBytecode/InternalBytecode.js:339:28)
    at tryCallOne (/Users/distiller/react-native/sdks/hermes/build_iphonesimulator/lib/InternalBytecode/InternalBytecode.js:53:16)
    at anonymous (/Users/distiller/react-native/sdks/hermes/build_iphonesimulator/lib/InternalBytecode/InternalBytecode.js:139:27)
    at apply (native)
    at anonymous (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26192:26)
    at _callTimer (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26111:17)
    at _callReactNativeMicrotasksPass (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26141:17)
    at callReactNativeMicrotasks (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:26304:44)
    at __callReactNativeMicrotasks (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19761:46)
    at anonymous (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19573:45)
    at __guard (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19745:15)
    at flushedQueue (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19572:21)
    at callFunctionReturnFlushedQueue (https://127.0.0.1:19000/index.bundle?platform=ios&dev=true&hot=false:19557:33)

Libs we are using:

    "expo": "~47.0.14",
    "sentry-expo": "~5.0.0",
    "@sentry/react-native": "4.2.2",
    "react-native": "0.70.8",

Identifying the offending line of code

I tried a few different StackOverflow answers for trying to find the real stack trace of the promise that was causing the issue I was seeing. As noted the stack trace that is printed does not end up leading to any code I have written, which made it questionable whether it was my fault or potentially some other library causing the issue.

After trying to use bluebird as suggested here I curiously found that the warning went away. Now I know it was because Bluebird promise implementation does not require a finally parameter!

In the end, I did the following to identify the offending line of code (sorry this isn't a patch-package patch as we do not use that lib See patch package below)

Manually add three lines to these two files

File 1: node_modules/promise/setimmediate/core.js
Add the following line of code in the function Promise(fn) {... function (line 61 for me).

  this._stack = (new Error()).stack;

File 2: node_modules/promise/setimmediate/rejection-tracking.js
Inside the enable function where a setTimeout is called (line 47 for me) with:

onUnhandled.bind(null, promise._51)

Replace it with:

onUnhandled.bind(null, promise._51, promise)

Then in the onUnhandled function just add a 2nd parameter "promise" and console log there and you'll see the _stack field printed out with the original location where the Promise was created.

  function onUnhandled(id, promise) { // line 60 for me
    console.log("rejection-tracking:61 - onUnhandled", id, promise)

Or use this Patch with `patch-package`

diff --git a/node_modules/promise/setimmediate/core.js b/node_modules/promise/setimmediate/core.js
index a84fb3d..1389b85 100644
--- a/node_modules/promise/setimmediate/core.js
+++ b/node_modules/promise/setimmediate/core.js
@@ -58,6 +58,14 @@ function Promise(fn) {
   if (typeof fn !== 'function') {
     throw new TypeError('Promise constructor\'s argument is not a function');
   }
+  if(!__DEV__) {
+    console.warn('WARNING - Promise Stack Trace Debugging is NOT recommended for production code.');
+  }
+  const stackTraceLimit = Error.stackTraceLimit
+  Error.stackTraceLimit = Infinity
+  this._stack = (new Error()).stack;
+  Error.stackTraceLimit = stackTraceLimit
+
   this._40 = 0;
   this._65 = 0;
   this._55 = null;
diff --git a/node_modules/promise/setimmediate/rejection-tracking.js b/node_modules/promise/setimmediate/rejection-tracking.js
index 10ccce3..75f2fd1 100644
--- a/node_modules/promise/setimmediate/rejection-tracking.js
+++ b/node_modules/promise/setimmediate/rejection-tracking.js
@@ -44,7 +44,7 @@ function enable(options) {
         displayId: null,
         error: err,
         timeout: setTimeout(
-          onUnhandled.bind(null, promise._51),
+          onUnhandled.bind(null, promise._51, promise),
           // For reference errors and type errors, this almost always
           // means the programmer made a mistake, so log them after just
           // 100ms
@@ -57,7 +57,8 @@ function enable(options) {
       };
     }
   };
-  function onUnhandled(id) {
+  function onUnhandled(id, promise) {
+    console.warn('Unhandled Promise Rejection', id, promise)
     if (
       options.allRejections ||
       matchWhitelist(

Although I got a bundled stack trace I was still able to track it down when the code was not minified.

NOTE: This is NOT performant and SHOULD NOT be included in any production builds as calling (new Error()).stack on every promise constructor will slow things down a lot. For debugging this is not an issue though.

I'd love for someone to tell me how I could have done this an alternative way as I feel it is ridiculous I had to go through these steps.

Edit: I decided to create a patch-package patch to make it easier to do this again in the future: