Bug fix: Console child wait for outstanding promises

[cliffoo]

core/lib/console-child.js should wait for outstanding promises to resolve before exiting, using promise-tracker, just like core/cli.js

This issue is surfaced in dashboard decoding PR, where the browser doesn't have the necessary compilations to decode so it complains by asking the user to truffle compile --all, and compile --all in a truffle console (connected to dashboard network) doesn't do anything, because the dashboard message bus client in the event system doesn't get a chance to send anything over the bus due to the process exiting prematurely.

@gnidan helped me debug this. It's improbable that I could dig this up myself.

[gnidan]

:D

[cds-amal]

@gnidan, What does waitForOutstandingPromisees() do here? It seems like a nop.

[gnidan]

It... waits for outstanding promises so we can process.exit() cleanly

[cds-amal]

It's called with no params, so, isn't iterable in the implementation just an empty array. Unless, I'm missing some setup in this scenario :/

export async function waitForOutstandingPromises<T extends Object>(
  options?: WaitForOutstandingPromiseOptions<T>
) {
  const { target } = options || {};
  let { catchHandler } = options || {};

  const iterable =
    (target
      ? _outstandingPromiseInstanceMap.get(target)?.values()
      : _outstandingPromiseMap.keys()) || [];

  catchHandler = catchHandler || (() => {});

  await Promise.all(Array.from(iterable, p => p.catch(catchHandler)));
}

[gnidan]

Yeah, hrm. Something weird must be implicitly happening, because this works and matches the usage in cli.js that @benjamincburns put there.

Ben, can you teach us?

[benjamincburns]

tl;dr: This is being used as expected here.

It’s called with no params, so, isn’t iterable in the implementation just an empty array?

When waitForOutstandingPromises is called with no arguments (as it should be in all non-test use cases), the value of iterable is only an empty array when there are no outstanding promises on which to wait.

The object destructuring assignment for target coupled with the ternary operator probably makes this code horrendously unreadable. Sorry about that. Allow me to break things down a bit...

First off, on the line where we initialize target:

const { target } = options || {};

We wind up with target being assigned undefined. This happens because options is undefined, so we fallback to the empty object literal and then attempt to destructure the target property out of that empty object literal. The empty object literal has no target property, so the destructuring assignment results with target being initialized with a value of undefined.

Given that detail, the following statement should be a bit more clear:

  const iterable =
    (target
      ? _outstandingPromiseInstanceMap.get(target)?.values()
      : _outstandingPromiseMap.keys()) || [];

The value of target is undefined, so we branch to the "else" statement in the ternary operator, which assigns the return value of _outstandingPromiseMap.keys() to the iterable variable. This always assigns that return value, as _outstandingPromiseMap is a module-scoped variable, and it's initialized as new Map<Promise<any>, boolean>() on module load (see code here).

If there's any further uncertainty, be sure to check the tests for the promise-tracker package as they're fairly comprehensive and they demonstrate and guard its usage nicely.

@gnidan @cds-amal @cliffoo please let me know if I can be of further assistance.

[cds-amal]

Thanks @benjamincburns, my question relates to how _outstandingPromiseMap is seeded with values. If nothing is inserted, then how does it know what to track? This PR only calls the tracking code (as a JS module) and doesn't have any code to set/identify/decorate what has to be awaited.

[benjamincburns]

There's a README.md, if that helps?

Otherwise the necessary code in question is already decorated. dashboard-message-bus-client depends on promise-tracker, and its internal send method is tracked here.

Decorating it with the @tracked label is all that's necessary to cause it to be tracked.

[benjamincburns]

Also if it helps any, this is the function that does the actual task tracking.

The tracked decorator injects this logic by returning a property descriptor that replaces the decorated method implementation with a closure over that impl that proxies calls to it and handles tracking of outstanding promises by wrapping any returned promises in special promise handlers that manage the state of _outstandingPromiseMap and friends.

[cds-amal]

Otherwise the necessary code in question is already decorated. dashboard-message-bus-client depends on promise-tracker, and its internal send method is tracked here.

Thanks @benjamincburns, this is the piece I was missing. It was unclear from the code that it relied on an already coded setup.

There should probably be a comment to explain this in the waiting portion, this PR.