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 outside of the iterable argument to Promise.allSettled,
// or safely wrap with the async keyword, Promise.try(), or Promise.resolve().then().

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 (!).

If the 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(syncParam()),
+   asyncY(param),
  ]);

Use the async keyword to denote asynchronous functions for safety; see the relevant section below for more information.

- const asyncX = () => {
+ const asyncX = async () => {
    // ...
  };

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

You can alternatively wrap the function invocation with Promise.try() or the slower Promise.resolve().then():

  Promise.all([
    asyncX(),
-   syncY(),
+   Promise.try(() => syncY()), // Node.js >=24
+   Promise.resolve().then(() => syncY()), // Node.js <=22
  ]);

By default, an unhandled promise rejection 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.

Limitations

This rule requires type checking and is heuristical. It may exit early without reporting issues in scenarios it has not accounted for.

Treat it as a helper to reduce the number of async issues in your codebase, not as a guarantee to flag all problematic occurrences.

Synchronous functions

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

// Never throws, but is still flagged
const syncParam = () => {
  // ...
};

const [x, y] = await Promise.all([asyncX(), asyncY(syncParam())]);
//                                                 ~~~~~~~~~~~
// syncParam() may synchronously throw an error and leave preceding promises dangling.
// Evaluate synchronous expressions outside of the iterable argument to Promise.all,
// or safely wrap with the async keyword, Promise.try(), or Promise.resolve().then().

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.

Asynchronous functions

TypeScript’s type system does not capture error handling, so this rule cannot exhaustively prove whether a thenable asyncX() function still throws synchronous errors.

The following example demonstrates the issue:

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

  return Promise.resolve();
}) satisfies () => Promise<void>; // While appearing asynchronous

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>;

You can also consider wrapping the function invocation with Promise.try() or the slower Promise.resolve().then():

// Node.js >=24
Promise.all([asyncX(), Promise.try(() => syncY())]);

// Node.js <=22
Promise.all([asyncX(), Promise.resolve().then(() => syncY())]);

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.