[fix] passing async functions to the Promise constructor

[spalger]

After fixing #109560 I did a little searching and found many cases where we are passing async functions to the Promise constructor without handling the potential of a rejected promise being created by said async function.

The following examples show the types of scenarios which are fixed by this PR:

---

Bad:

return () => 
  new Promise(async (resolve) => {
    await someAyncOperation();
    setImmediate(resolve); // or some other callback-based function
  })

In this example if someAsyncOperation() fails then the error will propagate from its call site to the promise created when the Promise() constructor called the async function. Unfortunately that promise is not accessible by user code so it will always result in an unhandled rejection. There are two ways to fix this, either ensure the hidden promise never rejects by wrapping the whole body of the function in a try/catch, or limit the code which is wrapped in the Promise constructor. I think the second approach is more ideal as it keeps the need for a resolve() callback as close to the source of that need as possible:

Fixed:

return async () => {
  await someAyncOperation();
  await new Promise((resolve) => setImmediate(resolve)); // or some other callback-based function
}

---

Bad:

const propValue = new Promise(async (resolve) => {
  const { prop } = await getObject();
  resolve(prop)
})

The goal of this function is to mutate the data coming back from the resolved promise in some way before returning/resolving to the outside caller. This should instead be done one of two ways:

Fix:

const propValue = (async () => {
  const { prop } = await getObject();
  return prop
})();

Using an async-IIFE is stylistically similar to the problematic code and properly routes errors in the case that getObject() rejects and handles possible synchronous errors, like getObject() not being a function, not returning a promise, or throwing synchronously for some reason.

---

Bad:

await new Promise(async (resolve, reject) => {
  const data = await loadSomeData();

  renderSomethingAndWaitForUserInteraction({
    data,
    onConfirm(selection) {
      resolve(selection);
    },
    onCancel() {
      reject()
    }
  })
})

Just like the previous example, if loadSomeData() rejects it's promise the hidden promise created by the Promise constructor will be rejected as the error propagates, but there is no way for us to handle that rejection. In this scenario I modeled the user interaction using an RxJS ReplaySubject(1), which is an Observable whose events can be programmatically customized from the outside and will remember the last value emitted and re-emit with new subscriptions. This might look unnecessarily complicated in my example but in the code where this pattern is used there are many things happening in the same place and the callbacks passed to renderSomethingAndWaitForUserInteraction() are not always constant, so this was a flexible way to model this scenario which has added benefits like being able to process each event produced on the Subject.

Fixed:

const selection$ = new Rx.ReplaySubject<SelectionType>(1);
const data = await loadSomeData();

renderSomethingAndWaitForUserInteraction({
  data,
  onConfirm(selection) {
    selection$.next(selection)
  },
  onCancel() {
    selection$.error(new Error('user canceled'))
  }
});

const selection = await firstValueFrom(selection$);

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 872fc70
• Storybooks Preview

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
indexManagement	1.3MB	1.3MB	-34.0B
ml	6.1MB	6.1MB	+780.0B
visTypeTable	104.0KB	104.2KB	+234.0B
visTypeVislib	564.9KB	565.0KB	+46.0B
total			+1.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
dataVisualizer	16.2KB	16.2KB	-18.0B
fileUpload	23.4KB	23.4KB	-12.0B
maps	79.3KB	79.2KB	-18.0B
mapsEms	10.6KB	10.6KB	-11.0B
ml	64.9KB	65.0KB	+66.0B
visTypeTable	8.2KB	8.4KB	+130.0B
total			+137.0B

Unknown metric groups

References to deprecated APIs

id	before	after	diff
ml	107	103	-4

History

• 💔 Build #147841 failed c0908a846c37011215cfa6fa05fe3a286f933e2c
• 💔 Build #147542 failed bf1093217467d57b9b855d11adb7c6bef6f1a50b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream