Patched bluebird does not provide data to promise chain

[jrabbe]

Just started investigating the upgrade of a large application based on AngularJS to Angular. This application relies on Bluebird as its global promise object and uses webpack as its module loader. The prescribed solutions mentioned in #455 were unsuccessful, as the promises from webpack kept resolving to undefined instead of the relevant view and controller content.

...
import './polyfills';

import * as Bluebird from 'bluebird';

import 'zone.js';
import 'zone.js/dist/zone-bluebird';

... 

// Patch bluebird to be zone aware. This also sets the global Promise object
// to be bluebird.
// Hackishly cast Zone so we can index it by the symbol for "bluebird" as
// patched by zone-bluebird.
(Zone as any)[Zone.__symbol__('bluebird')](Bluebird);

// Disable promise checking in zone.js because it doesn't recognize Bluebird as 
// zone aware, even when patched.
Zone.assertZonePatched = () => {};

...

After investigation the reason for these failures seem to be the way the promises are resolved in the bluebird patch function. The callbacks for .then, .spread, and .finally are patched to schedule the invocation as a zone.js micro task. However, the result from the callbacks is never made available to bluebird breaking the promise chain. Specifically, the call to zone.scheduleMicroTask returns a task, but that task is never saved, and the result of the task invocation is not passed back to Bluebird.

To solve this I have patched the Bluebird patch to return promises from the callbacks, ensuring Bluebird waits for the values to resolve when the micro task is invoked, and allowing the values to continue into the promise chain.

diff --git a/web/webpack/node_modules/zone.js/lib/extra/bluebird.ts b/web/webpack/node_modules/zone.js/lib/extra/bluebird.ts
index 13a0496eaa..0e8ef3b358 100644
--- a/web/webpack/node_modules/zone.js/lib/extra/bluebird.ts
+++ b/web/webpack/node_modules/zone.js/lib/extra/bluebird.ts
@@ -22,11 +22,21 @@ Zone.__load_patch('bluebird', (global: any, Zone: ZoneType, api: _ZonePrivate) =
           const func = args[i];
           if (typeof func === 'function') {
             args[i] = function() {
+              let callbackResolve, callbackReject;
+              const promise = new Bluebird((resolve, reject) => {
+                callbackResolve = resolve;
+                callbackReject = reject;
+              });
               const argSelf: any = this;
               const argArgs: any = arguments;
               zone.scheduleMicroTask('Promise.then', () => {
-                return func.apply(argSelf, argArgs);
+                try {
+                  callbackResolve(func.apply(argSelf, argArgs));
+                } catch (e) {
+                  callbackReject(e);
+                }
               });
+              return promise;
             };
           }
         }

[JiaLiPassion]

@jrabbe, I will check this one, could you post a test case to describe this issue?
In current version, this line https://github.com/angular/zone.js/blob/master/lib/extra/bluebird.ts#L34
will return the BluebirdPromise, so it should be able to be chained. I will add some test cases too.

[jrabbe]

You can chain bluebird promises, yes, but the value from the callbacks (each of the args) are not propagated. Because the call to func is wrapped in a function which schedules a micro task, the function (on line 25) immediately returns undefined. This results in two problems:

1. If the invocation of func returns a promise, which is a valid return from a .then callback, then the promise delegate could resolve too soon.

Promise.resolve()
  .then(() => console.log(Date.now()))
  .then(() => Promise.delay(1000))
  .then(() => console.log(Date.now()));

The difference between the two log statements should be roughly 1,000 ms, without the patch to return a promise mentioned in the original description, they both print almost immediately.

2. Because the return value of func is not propagated back to delegate it never receives the value and cannot propagate it to the promise chain.

Promise.resolve('a string')
  .then(value => {
    return { value };
  }).then(result => {
    console.log(result.value); // <-- will crash with "TypeError: cannot property 'value' of undefined"
  });

I will try and create a formal test case later today if I have time, but these two cases should definitely fail with the existing bluebird patch.

[JiaLiPassion]

@jrabbe, yes, you are right, your fix is correct, I will add test cases and fix this one, thank you very much for the research.

[Coobaha]

Hi @mhevery, I think this issue still persists.

I use knex.js that uses bluebird internally.

in my code i have

async function task() {
  try {
    await dbUpdateWithKnex();
  } catch (error) {}
}

error without patching bluebird is knex error
With zone-bluebird - error is undefined
Zone-bluebird with manually added catch to patched method - everything works
Without zone-bluebird transaction callback done within knex is loosing current zone context

[JiaLiPassion]

@Coobaha, could you provide a reproduce repo? Thanks.

[Coobaha]

@JiaLiPassion https://github.com/Coobaha/zone-bluebird-issue here you go

[Coobaha]

NOT PATCHED start
NOT PATCHED did error
Error: Unable to acquire a connection
    at Client_PG.acquireConnection (~/zonebluebird/node_modules/knex/lib/client.js:331:40)
    at Runner.ensureConnection (~/zonebluebird/node_modules/knex/lib/runner.js:228:24)
    at Runner.run (~/zonebluebird/node_modules/knex/lib/runner.js:34:42)
    at Builder.Target.then (~/zonebluebird/node_modules/knex/lib/interface.js:20:43)
    at processTicksAndRejections (internal/process/task_queues.js:86:5)
    at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:56:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:871:11)
    at internal/main/run_main_module.js:21:11
NOT PATCHED done
PATCHED start
~/zonebluebird/node_modules/zone.js/dist/zone-node.js:199
                        throw error;
                        ^

TypeError: Cannot read property 'release' of undefined
    at Client_PG.releaseConnection (~/zonebluebird/node_modules/knex/lib/client.js:348:32)
    at Object.<anonymous> (~/zonebluebird/node_modules/knex/lib/runner.js:236:28)
    at FunctionDisposer.doDispose (~/zonebluebird/node_modules/bluebird/js/release/using.js:98:19)
    at FunctionDisposer.Disposer.tryDispose (~/zonebluebird/node_modules/bluebird/js/release/using.js:78:20)
    at iterator (~/zonebluebird/node_modules/bluebird/js/release/using.js:36:53)
    at dispose (~/zonebluebird/node_modules/bluebird/js/release/using.js:48:9)
    at ~/zonebluebird/node_modules/bluebird/js/release/using.js:194:20
    at PassThroughHandlerContext.finallyHandler (~/zonebluebird/node_modules/bluebird/js/release/finally.js:56:23)
    at PassThroughHandlerContext.tryCatcher (~/zonebluebird/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (~/zonebluebird/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (~/zonebluebird/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (~/zonebluebird/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (~/zonebluebird/node_modules/bluebird/js/release/promise.js:694:18)
    at _drainQueueStep (~/zonebluebird/node_modules/bluebird/js/release/async.js:138:12)
    at _drainQueue (~/zonebluebird/node_modules/bluebird/js/release/async.js:131:9)
    at Async._drainQueues (~/zonebluebird/node_modules/bluebird/js/release/async.js:147:5)

It's not exactly the same issue i have in my project but seems to be related

[Coobaha]

If I add manually "catch" to zone-bluebird apis to patch it works

NOT PATCHED start
NOT PATCHED did error
Error: Unable to acquire a connection
    at Client_PG.acquireConnection (~/zonebluebird/node_modules/knex/lib/client.js:331:40)
    at Runner.ensureConnection (~/zonebluebird/node_modules/knex/lib/runner.js:228:24)
    at Runner.run (~/zonebluebird/node_modules/knex/lib/runner.js:34:42)
    at Builder.Target.then (~/zonebluebird/node_modules/knex/lib/interface.js:20:43)
    at processTicksAndRejections (internal/process/task_queues.js:86:5)
    at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:56:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:871:11)
    at internal/main/run_main_module.js:21:11
NOT PATCHED done
PATCHED start
PATCHED did error
{ Error: Unable to acquire a connection
    at Client_PG.acquireConnection (~/zonebluebird/node_modules/knex/lib/client.js:331:40)
    at Runner.ensureConnection (~/zonebluebird/node_modules/knex/lib/runner.js:228:24)
    at Runner.run (~/zonebluebird/node_modules/knex/lib/runner.js:34:42)
    at Builder.Target.then (~/zonebluebird/node_modules/knex/lib/interface.js:20:43) sql: undefined, bindings: undefined }
PATCHED done