oven-sh/bun

Allow asserting uncaught errors in the Bun's test runner

Opened this issue · 4 comments

What is the problem this feature would solve?

It is currently impossible (to my knowledge) to assert a tested function throws an exception after it returns:

async function demo() {
  const { promise, resolve, reject } = Promise.withResolvers();

  setImmediate(() => {
    resolve(42);
  });

  setImmediate(() => {
    throw new Error('Uncaught!');
  });

  return await promise;
}

We can test it:

test('returns and then throws', async () => {
  const result = await demo();
  expect(result).toBe(42);
});

But the test runner will still show the uncaught error even though the test will pass.

What is the feature you are proposing to solve the problem?

Add some sort of an API to catch these. I tried using:

process.on('unhandledRejection', (error) => {
  console.log('Caught!');
});

I have a repro here: https://github.com/TomasHubelbauer/bun-throw-after-return

What alternatives have you considered?

No response

What if you do

expect(() => func()).toThrow()

I tried this in my repro repo to no avail.

What about

expect(async () => func()).toThrow()

We override the exception handler in .toThrow when given an async function and drain microtasks to try to catch this sort of thing

setImmediate is particularly difficult though as that runs later

I also tried that and similarly it didn't work.

BTW I am using setImmediate in the repro to convey the problem is that the throw happens on the next tick, but my real method is this:

const DEFAULT_OPTIONS = {
  throwOnError: true,
  throwOnMultiple: true
};

/**
 * @overload
 * @param {EventTarget} eventTarget 
 * @param {Partial<typeof DEFAULT_OPTIONS>} options 
 * @param  {...string[]} names 
 * @returns {Promise<{name: string, event: Event}>}
 * 
 * @overload
 * @param {EventTarget} eventTarget
 * @param {...string[]} names
 * @returns {Promise<{name: string, event: Event}>}
 */
export default async function race(
  eventTarget,
  options = DEFAULT_OPTIONS,
  ...names
) {
  if (typeof options === 'string') {
    names = [options, ...names];
    options = DEFAULT_OPTIONS;
  }

  const { throwOnError, throwOnMultiple } = { ...DEFAULT_OPTIONS, ...options };

  let settledName;
  const { promise, resolve, reject } = Promise.withResolvers();
  for (const name of names) {
    eventTarget.addEventListener(name, (event) => {
      if (settledName && throwOnMultiple) {
        throw new Error(`Event '${name}' was called after the deferred promise already settled for the '${settledName}' event.`);
      }

      settledName = name;
      if (throwOnError && name === 'error') {
        reject(event);
      }
      else {
        resolve({ name, event });
      }
    }, { once: true });
  }

  return await promise;
}

And my test is this:

test('default throwOnMultiple', async () => {
  const firstEvent = new Event('first');
  const secondEvent = new Event('second');
  const eventTarget = makeEventTarget(firstEvent, secondEvent);

  // TODO: Find a way to handle the uncaught error and remove the `.not` here
  // See https://github.com/TomasHubelbauer/bun-throw-after-return for repro
  // See https://github.com/oven-sh/bun/issues/10226 for feature request
  // See https://twitter.com/tomashubelbauer/status/1778881576912183670 tweet
  expect(async () => {
    const result = await race(eventTarget, 'first', 'second');
    expect(result.name).toBe('first');
    expect(result.event).toBe(firstEvent);
  }).not.toThrow();
});

function makeEventTarget(...events: Event[]) {
  const eventTarget = new EventTarget();

  // Dispatch on the next tick so `race` can assign event handles first
  for (const event of events) {
    setImmediate(() => { eventTarget.dispatchEvent(event); });
  }

  return eventTarget;
}

But I think whatever the mechanism, it should work the same. Basically, in my mind, there should be a wait to tell the test that not only should it run through its assertions, but also somehow indicate it should wait (potentially up until the default timeout?) for next-tick effects like this setImmediate or the thing with event handlers I am doing.

WDYT, feasible? Worth it?