fix: Promise.prototype.finally() must passthrough when onFinally is not callable

[dan-slater]

Summary

Promise.prototype.finally() in Hermes rejects with TypeError: undefined is not a function when onFinally is not callable. Per ECMA-262 §27.2.5.3, a non-callable onFinally must be treated as a passthrough — thenFinally and catchFinally are set to onFinally itself (steps 6.a and 7.a), so then(onFinally, onFinally) mirrors the receiver's value/rejection.

This was discovered in a React Native 0.85.3 app where calling .finally() on a fetch promise crashed on cold start. The same call works in every other engine.

Minimal repro

// hermes -Xes6-promise
Promise.resolve('ok').finally().then(print, e => print('REJECTED', e.message));
// before: REJECTED undefined is not a function
// after:  ok

For comparison, V8 / JSC / SpiderMonkey:

$ node -e "Promise.resolve('ok').finally().then(console.log)"
ok

Cause

lib/InternalBytecode/01-Promise.js is generated from the promise@8.3.0 npm polyfill, which calls f() unconditionally:

core.prototype.finally = function (f) {
  return this.then(function (value) {
    return core.resolve(f()).then(function () { return value; });
  }, function (err) {
    return core.resolve(f()).then(function () { throw err; });
  });
};

f() throws when f is undefined / null / any non-callable.

Fix

If onFinally is not a function, forward it straight to this.then(f, f). This is exactly what the spec describes when IsCallable(onFinally) is false, and it matches what Hermes already does on the static_h branch (lib/InternalJavaScript/01-Promise.js):

if (typeof onFinally !== 'function') {
  thenFinally = onFinally;
  catchFinally = onFinally;
}
// ...
return this.then(thenFinally, catchFinally);

The diff to the generated polyfill is a 9-line guard:

if (typeof f !== 'function') {
  return this.then(f, f);
}

Before / after

Call	Engine spec	Hermes (before)	Hermes (after)
Promise.resolve('ok').finally()	resolves 'ok'	rejects TypeError	resolves 'ok'
Promise.resolve('ok').finally(undefined)	resolves 'ok'	rejects TypeError	resolves 'ok'
Promise.resolve('ok').finally(null)	resolves 'ok'	rejects TypeError	resolves 'ok'
Promise.reject(new Error('x')).finally()	rejects Error('x')	rejects TypeError	rejects Error('x')
Promise.resolve('ok').finally(() => {})	resolves 'ok'	resolves 'ok'	resolves 'ok'

Tests

Added test/hermes/promise-finally.js covering all five rows above. It follows the same lit + FileCheck convention as the existing test/hermes/promise.js and runs under the same three configurations (-Xes6-promise, -Xmicrotask-queue, and via hermesc -emit-binary).

I was unable to run check-hermes locally in this environment (system libicu-dev isn't installed, so CMake configuration fails before building). I verified the patched logic in isolation against V8 (Node) and the outputs match in all five cases. CI here will confirm against the real engine.

Scope

• lib/InternalBytecode/01-Promise.js — 9-line guard added to the finally polyfill.
• test/hermes/promise-finally.js — new file, covers the spec passthrough cases plus a no-regression sanity check.

No other files touched. No formatting / lint / copyright changes elsewhere. The fix on the static_h branch is already correct, so no change needed there.

Notes

I see the project requires a CLA — I'll sign it when CLA-bot prompts.

[meta-cla]

Hi @dan-slater!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[tmikov]

Hi, our main branch (legacy Hermes) is frozen we don't accept PRs to it, except critical security fixes. Does your fix apply to the static_h branch?

[lavenzg]

Hi, our main branch (legacy Hermes) is frozen we don't accept PRs to it, except critical security fixes. Does your fix apply to the static_h branch?

This has been fixed in static_h (along with other fixes).