Promise with "chained" finally gobbles return value

[pstanton]

let promise = new Promise(...).finally();
promise.then();

behaves differently to

let promise = new Promise(...);
promise.finally();
promise.then();

If the finally is chained (first eg) any "then" or "catch" subsequently added will not receive the value from "resolve" "reject" "return" or "throw" that was above the "finally" in the chain.

Is this a bug report?

yes

Have you read the Contributing Guidelines?

kind of

Environment

Environment:
OS: Windows 10
Node: 9.0.0
Yarn: 1.3.2
npm: 5.5.1
Watchman: Not Found
Xcode: N/A
Android Studio: Version 3.0.0.0 AI-171.4408382

Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.53.0 => 0.53.0

Steps to Reproduce

Run the sample code, observe the log results.

Expected Behavior

According to the documentation for promises that I've read, the two styles of implementation should be equal. Chaining the operations should be equivalent to putting each one on it's own line.

When the same code is run in a browser (using q), both versions produce the same (expected) result, in that the internal return/throw value is propagated out to the external then/catch.

TEST>--------------------------------------
TEST>Test Promise chained succeed
TEST>internal then success
TEST>internal finally
TEST>external then success
TEST>--------------------------------------
TEST>Test Promise chained fail
TEST>internal catch error
TEST>internal finally
TEST>external then error
TEST>--------------------------------------
TEST>Test Promise unchained succeed
TEST>internal then success
TEST>internal finally
TEST>external then success
TEST>--------------------------------------
TEST>Test Promise unchained fail
TEST>internal catch error
TEST>internal finally
TEST>external catch error

Actual Behavior

TEST>--------------------------------------
TEST>Test Promise chained succeed
TEST>internal then success
TEST>internal finally
TEST>external then undefined
TEST>--------------------------------------
TEST>Test Promise chained fail
TEST>internal catch error
TEST>internal finally
TEST>external then undefined
TEST>--------------------------------------
TEST>Test Promise unchained succeed
TEST>internal then success
TEST>internal finally
TEST>external then success
TEST>--------------------------------------
TEST>Test Promise unchained fail
TEST>internal catch error
TEST>internal finally
TEST>external catch error

(Write what happened. Add screenshots!)

Reproducible Demo

function testPromise(succeed) {
	console.log("TEST>--------------------------------------");
	console.log("TEST>Test Promise chained", succeed ? "succeed" : "fail");
	let promise = new Promise(
		function(resolve, reject) {
			if (succeed) resolve("success");
			else reject("error");
		}.bind(this)
	)
		.then(
			function(result) {
				console.log("TEST>internal then", result);
				return result;
			}.bind(this)
		)
		.catch(
			function(error) {
				console.log("TEST>internal catch", error);
				throw error; // bubble up
			}.bind(this)
		)
		// finally is chained
		.finally(
			function() {
				console.log("TEST>internal finally");
			}.bind(this)
		);

	// pretend the promise object is returned to the caller, and it adds 'then' and 'catch' below
	promise
		.then(
			function(result) {
				console.log("TEST>external then", result);
			}.bind(this)
		)
		.catch(
			function(error) {
				console.log("TEST>external catch", error);
			}.bind(this)
		);
}

function testPromise2(succeed) {
	console.log("TEST>--------------------------------------");
	console.log("TEST>Test Promise unchained", succeed ? "succeed" : "fail");
	let promise = new Promise(
		function(resolve, reject) {
			if (succeed) resolve("success");
			else reject("error");
		}.bind(this)
	)
		.then(
			function(result) {
				console.log("TEST>internal then", result);
				return result;
			}.bind(this)
		)
		.catch(
			function(error) {
				console.log("TEST>internal catch", error);
				throw error; // bubble up
			}.bind(this)
		);

	// finally is not chained
	promise.finally(
		function() {
			console.log("TEST>internal finally");
		}.bind(this)
	);

	// pretend the promise object is returned to the caller, and it adds 'then' and 'catch' below
	promise
		.then(
			function(result) {
				console.log("TEST>external then", result);
			}.bind(this)
		)
		.catch(
			function(error) {
				console.log("TEST>external catch", error);
			}.bind(this)
		);
}
testPromise(true);
setTimeout(function() {
	testPromise(false);
	setTimeout(function() {
		testPromise2(true);
		setTimeout(function() {
			testPromise2(false);
		}, 1000);
	}, 1000);
}, 1000);

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

[pstanton]

@hramos i think this is important to keep open .. yes?

[hramos]

Sure. Looks like this needs to be addressed in fbjs itself, and there's a PR waiting for review that may fix this: facebook/fbjs#293

[hramos]

Looks like this behavior is intentional: facebook/fbjs#95

[grabbou]

Closing as this is fbjs issue. Let's follow up there and hopefully, get this merged soon.