no-sync-in-promise-iterable

{
  'skuba/no-sync-in-promise-iterable': 'warn',
}

Heuristically flags synchronous logic in the iterable argument of static Promise methods that could leave preceding promises dangling.

const [x, y] = await Promise.allSettled([asyncX(), syncY()]);
//                                                 ~~~~~~~
// syncY() may synchronously throw an error and leave preceding promises dangling.
// Evaluate synchronous expressions before constructing the iterable argument to Promise.allSettled.

Problem

In the above example, if both asyncX() and syncY() throw errors, the former will result in an unhandled promise rejection. By default, this will cause the Node.js process to exit (!).

The resilience of back-end applications can be improved by changing the --unhandled-rejections CLI mode or adding a process.on('unhandledRejection') event handler to avoid this terminal state. Addressing the underlying issue is still useful to avoid dispatching unnecessary promises which may degrade performance or trigger unanticipated partial failure states.

If the above example felt contrived, consider that the issue is not limited to synchronous functions that are directly passed as an iterable element to a static Promise method. You may have an argument or condition that implicates synchronous evaluation:

const [x, y] = await Promise.all([
  asyncX(),
  condition
    ? asyncY()
    : // May throw a synchronous error
      syncFallback(),
]);

const [x, y] = await Promise.all([
  asyncX(),
  asyncY(
    // May throw a synchronous error
    syncParam(),
  ),
]);

This rule targets static Promise methods as typical callsites where the issue crops up. However, the underlying cause is more fundamentally about asynchronous programming in JavaScript and order of evaluation. For example, you could equally construct the following:

const [promiseX, y] = [
  asyncX(), // Queues up work to throws an error
  syncY(), // Immediately throws an error
];

const x = await promiseX; // Never reached, so `promiseX` rejection is unhandled

The above can be expanded to the following:

const first = asyncX(); // Queues up work to throws an error
const second = syncY(); // Immediately throws an error

const [promiseX, y] = [first, second];

const x = await promiseX; // Never reached, so `promiseX` rejection is unhandled

This is an issue for non-head elements in the iterable. Array elements are evaluated in the order they are specified, so an error thrown during evaluation of the head element will not leave preceding promises dangling.

const [y, promiseX] = [
  syncY(), // It's okay to throw before we make any promises
  asyncX(), // Never reached, so no promise is left dangling
];

const x = await promiseX;

Fix

Avoid invoking synchronous logic between an async function being dispatched (asyncX()) and its promise being handled (by a static Promise method, await, etc).

const param = syncParam(); // It's okay to throw before we make any promises

await Promise.all([asyncX(), asyncY(param)]);

Limitations

This rule requires type checking.

False positives

TypeScript’s type system does not capture error handling, so this rule assumes that a given syncY() function may throw and should be evaluated before constructing the iterable argument.

// Never throws, but is still flagged
const syncY = () => undefined;

const [x, y] = await Promise.allSettled([asyncX(), syncY()]);
//                                                 ~~~~~~~
// syncY() may synchronously throw an error and leave preceding promises dangling.
// Evaluate synchronous expressions before constructing the iterable argument to Promise.allSettled.

We recommend restructuring your code regardless to avoid this class of issue; consider that you may change the implementation of syncY() to throw an error in future.

Synchronous errors from asynchronous functions

TypeScript’s type system does not capture error handling, so this rule assumes that a given asyncX() function with a thenable return type will only throw asynchronous errors.

The assumption is not strictly correct. Consider the following:

const evil = (() => {
  if (condition) {
    throw new Error('Synchronous error');
  }

  return Promise.resolve();
}) satisfies () => Promise<void>;

Use of the async keyword can help to ensure the whole function evaluation is deferred, and can be enforced with the @typescript-eslint/promise-function-async rule. It comes with performance implications, but you are unlikely to need to micro-optimise to this degree.

const good = (async () => {
  if (something) {
    throw new Error('Asynchronous error');
  }

  return;
}) satisfies () => Promise<void>;

Other options like Promise.try() may be explored in future.

Getters and setters

TypeScript’s type system does not capture error handling, so this rule assumes that property access and assignment is safe.

The assumption is not strictly correct. Custom getters and setters can throw errors:

const obj = {
  get prop() {
    throw new Error('Badness!');
  },
};

obj.prop;
// Uncaught Error: Badness!

The no-restricted-syntax rule can flag custom getters and setters:

module.exports = [
  rules: {

  'no-restricted-syntax': [
    ERROR,
    {
      selector: 'MethodDefinition[kind = "get"]',
      message:
        'Custom getters can cause confusion, particularly if they throw errors. Remove the `get` syntax to specify a regular method instead.',
    },
    {
      selector: 'MethodDefinition[kind = "set"]',
      message:
        'Custom setters can cause confusion, particularly if they throw errors. Remove the `set` syntax to specify a regular method instead.',
    },
    {
      selector: 'Property[kind = "get"]',
      message:
        'Custom getters can cause confusion, particularly if they throw errors. Remove the `get` syntax to specify a regular property instead.',
    },
    {
      selector: 'Property[kind = "set"]',
      message:
        'Custom setters can cause confusion, particularly if they throw errors. Remove the `set` syntax to specify a regular property instead.',
    },],
  }
];

This configuration is baked in to eslint-config-seek and eslint-config-skuba.