From 60e3c5c0d851f75310456eaed8ff9247440760c6 Mon Sep 17 00:00:00 2001 From: "Daniel Slater (autodev)" Date: Sat, 23 May 2026 14:13:37 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20Promise.prototype.finally()=20must=20not?= =?UTF-8?q?=20throw=20on=20non-callable=20onFinally=20(spec=20=C2=A727.2.5?= =?UTF-8?q?.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per ECMA-262 §27.2.5.3 (Promise.prototype.finally), when IsCallable(onFinally) is false the value/rejection of the receiver must pass straight through — steps 6.a and 7.a let `thenFinally`/`catchFinally` be `onFinally` itself, so the result mirrors the original Promise. All major engines implement this: $ node -e "Promise.resolve('ok').finally().then(console.log)" ok # V8 (Node 10+ / Chrome 63+), JavaScriptCore (Safari 11.1+), # SpiderMonkey (Firefox) all agree. The Hermes implementation in lib/InternalBytecode/01-Promise.js (generated from the `promise@8.3.0` polyfill, which inherits this bug) instead invokes `f()` unconditionally, so: Promise.resolve('ok').finally() // rejects: undefined is not a function Promise.resolve('ok').finally(undefined) // rejects: undefined is not a function Promise.resolve('ok').finally(null) // rejects: null is not a function The user-visible symptom that surfaced this was a React Native 0.85.3 app crashing on cold start with "TypeError: undefined is not a function" coming from app code that called `.finally()` on a fetch promise — code that runs fine in every other JS environment. The fix mirrors what Hermes already does on the `static_h` branch (lib/InternalJavaScript/01-Promise.js): when `onFinally` is not a function, forward it straight to `this.then(onFinally, onFinally)`, which is the shortest faithful implementation of the spec's passthrough closures. Minimal reproducer (run with `hermes -Xes6-promise`): Promise.resolve('ok').finally().then(print, e => print('REJECTED', e.message)); // before: REJECTED undefined is not a function // after: ok Added test/hermes/promise-finally.js covering the four spec passthrough cases (no-arg / undefined / null / rejection) plus a sanity check that callable onFinally still works. Co-Authored-By: Claude Opus 4.7 --- lib/InternalBytecode/01-Promise.js | 9 ++++++ test/hermes/promise-finally.js | 49 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 test/hermes/promise-finally.js diff --git a/lib/InternalBytecode/01-Promise.js b/lib/InternalBytecode/01-Promise.js index ea28bd2da99..6300892b0d3 100644 --- a/lib/InternalBytecode/01-Promise.js +++ b/lib/InternalBytecode/01-Promise.js @@ -397,6 +397,15 @@ }; core.prototype.finally = function (f) { + // Per ECMA-262 §27.2.5.3 (Promise.prototype.finally), when onFinally + // is not callable it must be passed straight through to then(), so + // the resulting Promise simply mirrors the receiver. Without this + // guard, calling f() below throws "undefined is not a function" for + // the no-arg / undefined / null cases that V8, JavaScriptCore and + // SpiderMonkey all accept as passthrough. + if (typeof f !== 'function') { + return this.then(f, f); + } return this.then(function (value) { return core.resolve(f()).then(function () { return value; diff --git a/test/hermes/promise-finally.js b/test/hermes/promise-finally.js new file mode 100644 index 00000000000..44e6eea5dc9 --- /dev/null +++ b/test/hermes/promise-finally.js @@ -0,0 +1,49 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermes -Xes6-promise %s | %FileCheck --match-full-lines %s +// RUN: %hermes -Xmicrotask-queue %s | %FileCheck --match-full-lines %s +// RUN: %hermesc -O -emit-binary -out %t.hbc %s && %hermes -Xes6-promise %t.hbc | %FileCheck --match-full-lines %s + +// Regression test for Promise.prototype.finally with a non-callable +// onFinally. Per ECMA-262 §27.2.5.3, when IsCallable(onFinally) is false +// the value/rejection of the receiver must pass straight through. + +print('promise-finally'); +// CHECK-LABEL: promise-finally + +// 1) No-arg call: value passes through. +Promise.resolve('ok').finally().then(function (v) { + print('no-arg:', v); +}); +// CHECK-NEXT: no-arg: ok + +// 2) Explicit undefined: value passes through. +Promise.resolve('ok').finally(undefined).then(function (v) { + print('undefined:', v); +}); +// CHECK-NEXT: undefined: ok + +// 3) null: value passes through. +Promise.resolve('ok').finally(null).then(function (v) { + print('null:', v); +}); +// CHECK-NEXT: null: ok + +// 4) Rejection passes through unchanged when onFinally is not callable. +Promise.reject(new Error('x')).finally().then(function () { + print('rejection: unexpected resolve'); +}, function (e) { + print('rejection:', e.message); +}); +// CHECK-NEXT: rejection: x + +// 5) Sanity: callable onFinally still works (no regression). +Promise.resolve('ok').finally(function () {}).then(function (v) { + print('callable:', v); +}); +// CHECK-NEXT: callable: ok