diff --git a/CHANGELOG.md b/CHANGELOG.md index 088f82a4f..214ddfa2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file. The format ## [Unreleased] +### Fixed + +- Fix a component whose lifecycle hook throws synchronously wedging the shared task queue and stopping every other component on the page from mounting — the batch was aborted, its remaining tasks orphaned and the scheduling flag left stuck, so no later batch was ever flushed ([#749](https://github.com/studiometa/js-toolkit/pull/749)) +- Fix `Queue.add()` never settling when its task throws synchronously — the returned promise now rejects with the error, exactly like an `async` task whose promise rejects ([#749](https://github.com/studiometa/js-toolkit/pull/749)) +- Fix `registerComponent()` returning an instance that failed to mount as a successful registration — instances that are not mounted are skipped again, as documented in v3.8.0 ([#749](https://github.com/studiometa/js-toolkit/pull/749)) + +### Changed + +- `$mount()`, `$update()`, `$destroy()` and `$terminate()` no longer reject when a lifecycle hook fails: they run detached on a shared queue and are commonly called fire-and-forget, so a rejection had no awaiter. The error is re-thrown from a microtask instead, keeping it visible to `window.onerror` and error monitors, wrapped with the instance `$id` and the failed lifecycle and carrying the original error as its `cause`. `after-mounted` and `after-destroyed` are still not emitted on failure ([#749](https://github.com/studiometa/js-toolkit/pull/749)) +- With the `blocking` feature enabled the lifecycle methods keep rejecting — no queue is involved there, so `try { await createApp(App, { blocking: true }) } catch {}` still catches startup failures ([#749](https://github.com/studiometa/js-toolkit/pull/749)) + ## [v3.8.2](https://github.com/studiometa/js-toolkit/compare/3.8.1..3.8.2) (2026-08-06) ### Fixed diff --git a/packages/docs/api/instance-methods.md b/packages/docs/api/instance-methods.md index fe01de0d0..52add0aea 100644 --- a/packages/docs/api/instance-methods.md +++ b/packages/docs/api/instance-methods.md @@ -277,3 +277,13 @@ Terminate the component. Its instance becomes available for garbage collection. :::warning A terminated component can not be re-mounted, use with precaution. ::: + +## Error handling + +Lifecycle methods run their work through a shared task queue, and they are commonly called fire-and-forget (`$mount()` from the auto-mounting mutation observer, `$terminate()` when an element leaves the DOM). A rejection there would have no awaiter, so `$mount()`, `$update()`, `$destroy()` and `$terminate()` **resolve even when a lifecycle hook throws**. + +The error is not swallowed: it is re-thrown from a microtask, so it reaches `window.onerror` and any error monitor, wrapped in an error naming the instance and the failed lifecycle, with the original error as its `cause`. + +A failed lifecycle stops at the point of failure: `after-mounted` and `after-destroyed` are not emitted, and a component whose wiring failed stays unmounted. A component whose `mounted()` hook threw is wired, so `$isMounted` is `true` and `$destroy()` can still tear it down. + +With the [`blocking` feature](./helpers/createApp.md) enabled no queue is involved — the work runs synchronously in the caller's stack — so the lifecycle methods keep rejecting and `try { await createApp(App, { blocking: true }) } catch {}` still catches startup failures. diff --git a/packages/docs/utils/Queue.md b/packages/docs/utils/Queue.md index dcf39fb04..e37c8512f 100644 --- a/packages/docs/utils/Queue.md +++ b/packages/docs/utils/Queue.md @@ -17,3 +17,30 @@ queue.add(() => console.log('2')); - `concurrency` (`Number`): the number of tasks to execute at the same time, defaults to `10` - `waiter` (`(cb: (...args:unknown[]) => unknown) => unknown`): a scheduler function for the next batch execution, defaults to an immediately invoked function `(cb) => cb()` + +## `add(task)` + +Add a task to the queue. + +**Return value** + +- `Promise`: a promise resolving with the value returned by the task. + +The promise **rejects when the task fails**, whether it throws synchronously or returns a rejected promise. The failure is contained: the other tasks of the batch still run, and the queue keeps scheduling later batches. + +:::warning +A task that can fail must have its rejection observed, otherwise it becomes an unhandled rejection. + +```js +// Fire-and-forget is fine for a task that can not fail. +queue.add(() => console.log('1')); + +// Otherwise, await the promise or attach a `catch`. +try { + await queue.add(() => mightThrow()); +} catch (error) { + // Handle the failure. +} +``` + +::: diff --git a/packages/js-toolkit/Base/Base.ts b/packages/js-toolkit/Base/Base.ts index 533b56133..36eeca81e 100644 --- a/packages/js-toolkit/Base/Base.ts +++ b/packages/js-toolkit/Base/Base.ts @@ -7,6 +7,7 @@ import { deleteInstance, addToRegistry, hasInstance, + handleLifecycleError, } from './utils.js'; import { ChildrenManager, @@ -474,18 +475,29 @@ export class Base { this.__debug('$mount'); } - await Promise.all([ - addToQueue(() => this.__children.registerAll()), - addToQueue(() => this.__refs.registerAll()), - addToQueue(() => this.__events.bindRootElement()), - addToQueue(() => this.__services.enableAll()), - addToQueue(() => this.__children.mountAll()), - ]); - - await addToQueue(() => { - this.$isMounted = true; - this.__callMethod('mounted'); - }); + try { + await Promise.all([ + addToQueue(() => this.__children.registerAll()), + addToQueue(() => this.__refs.registerAll()), + addToQueue(() => this.__events.bindRootElement()), + addToQueue(() => this.__services.enableAll()), + addToQueue(() => this.__children.mountAll()), + ]); + + await addToQueue(() => { + this.$isMounted = true; + this.__callMethod('mounted'); + }); + } catch (error) { + // A queued mount task failed. Do not emit `after-mounted` as if init had + // completed, and leave `$isMounted` at whatever value it reached: `false` + // when the failure happened while wiring, `true` when the `mounted()` hook + // itself threw — the component is wired in that case, and `$destroy()` + // relies on the flag to tear it down. + this.__isMounting = false; + handleLifecycleError(this, '$mount', error); + return this; + } this.__isMounting = false; this.$emit('after-mounted'); @@ -502,19 +514,23 @@ export class Base { this.__debug('$update'); } - await Promise.all([ - // Undo - addToQueue(() => this.__refs.unregisterAll()), - addToQueue(() => this.__services.disableAll()), - // Redo - addToQueue(() => this.__children.registerAll()), - addToQueue(() => this.__refs.registerAll()), - addToQueue(() => this.__services.enableAll()), - // Update - addToQueue(() => this.__children.updateAll()), - ]); - - await addToQueue(() => this.__callMethod('updated')); + try { + await Promise.all([ + // Undo + addToQueue(() => this.__refs.unregisterAll()), + addToQueue(() => this.__services.disableAll()), + // Redo + addToQueue(() => this.__children.registerAll()), + addToQueue(() => this.__refs.registerAll()), + addToQueue(() => this.__services.enableAll()), + // Update + addToQueue(() => this.__children.updateAll()), + ]); + + await addToQueue(() => this.__callMethod('updated')); + } catch (error) { + handleLifecycleError(this, '$update', error); + } return this; } @@ -535,14 +551,22 @@ export class Base { this.$emit('before-destroyed'); this.$isMounted = false; - await Promise.all([ - addToQueue(() => this.__events.unbindRootElement()), - addToQueue(() => this.__refs.unregisterAll()), - addToQueue(() => this.__services.disableAll()), - addToQueue(() => this.__children.destroyAll()), - ]); - - await addToQueue(() => this.__callMethod('destroyed')); + try { + await Promise.all([ + addToQueue(() => this.__events.unbindRootElement()), + addToQueue(() => this.__refs.unregisterAll()), + addToQueue(() => this.__services.disableAll()), + addToQueue(() => this.__children.destroyAll()), + ]); + + await addToQueue(() => this.__callMethod('destroyed')); + } catch (error) { + // A queued teardown task failed. Do not emit `after-destroyed` (which + // removes the instance from the global storage) as if teardown had + // completed. + handleLifecycleError(this, '$destroy', error); + return this; + } this.$emit('after-destroyed'); @@ -558,14 +582,18 @@ export class Base { this.__debug('$terminate'); } - await Promise.all([ - // First, destroy the component. - addToQueue(() => this.$destroy()), - // Execute the `terminated` hook if it exists - addToQueue(() => this.__callMethod('terminated')), - // Delete instance - addToQueue(() => this.$el.__base__.set(this.$config.name, 'terminated')), - ]); + try { + await Promise.all([ + // First, destroy the component. + addToQueue(() => this.$destroy()), + // Execute the `terminated` hook if it exists + addToQueue(() => this.__callMethod('terminated')), + // Delete instance + addToQueue(() => this.$el.__base__.set(this.$config.name, 'terminated')), + ]); + } catch (error) { + handleLifecycleError(this, '$terminate', error); + } } /** diff --git a/packages/js-toolkit/Base/managers/ChildrenManager.ts b/packages/js-toolkit/Base/managers/ChildrenManager.ts index 47d250410..72ce57a3a 100644 --- a/packages/js-toolkit/Base/managers/ChildrenManager.ts +++ b/packages/js-toolkit/Base/managers/ChildrenManager.ts @@ -1,6 +1,6 @@ import type { Base, BaseConstructor, BaseAsyncConstructor, BaseEl } from '../index.js'; import { AbstractManager } from './AbstractManager.js'; -import { getComponentElements, addToQueue } from '../utils.js'; +import { getComponentElements, addToQueue, reportQueuedTaskError } from '../utils.js'; /** * Children manager. @@ -204,9 +204,20 @@ export class ChildrenManager extends AbstractManager { for (const name of this.registeredNames) { for (const instance of this.props[name]) { if (instance instanceof Promise) { - instance.then((resolvedInstance) => - addToQueue(() => this.__triggerHook(hook, resolvedInstance, name)), - ); + // Fire-and-forget: this branch does not push into `promises`, so its + // result is never awaited. Attach a `.catch()` so a failure in the + // async child resolution or its queued hook reaches the global error + // handler instead of becoming an unhandled rejection. + instance + .then((resolvedInstance) => + addToQueue(() => this.__triggerHook(hook, resolvedInstance, name)), + ) + .catch((error) => + reportQueuedTaskError( + error, + `[${this.__base.$id}] The \`${hook}\` hook failed for an async \`${name}\` child.`, + ), + ); } else { promises.push(addToQueue(() => this.__triggerHook(hook, instance, name))); } diff --git a/packages/js-toolkit/Base/utils.ts b/packages/js-toolkit/Base/utils.ts index 642647568..2ce5e506f 100644 --- a/packages/js-toolkit/Base/utils.ts +++ b/packages/js-toolkit/Base/utils.ts @@ -22,6 +22,40 @@ export function addToQueue(fn: () => unknown) { return queue.add(fn); } +/** + * Surface an error raised by a queued task. + * + * Queued work runs detached from its original call site — fire-and-forget + * mounts, mutation-driven auto-mounting, async children — so the promise + * returned by `addToQueue` is usually discarded and a rejection would be lost. + * Re-throwing from a microtask puts the failure back on the global error + * channel (`window.onerror`, `uncaughtException`, error monitors) without + * wedging the queue or the caller's control flow. The original error is kept as + * the `cause`, so its stack and type survive; `context` adds the component + * identity a bare rejection can not carry. + */ +export function reportQueuedTaskError(error: unknown, context: string): void { + queueMicrotask(() => { + throw new Error(context, { cause: error }); + }); +} + +/** + * Handle an error raised by a queued lifecycle task. + * + * In blocking mode `addToQueue` runs the task synchronously in the caller's own + * stack — no queue, nothing to wedge — so the error is re-thrown and the + * lifecycle promise rejects exactly as it always has. That keeps + * `try { await createApp(App, { blocking: true }) } catch` working. + */ +export function handleLifecycleError(instance: Base, phase: string, error: unknown): void { + if (features.get('blocking')) { + throw error; + } + + reportQueuedTaskError(error, `[${instance.$id}] The \`${phase}\` lifecycle failed.`); +} + const selectors = new Map(); // Separator used for multi-component declaration in `data-component` attributeS. @@ -181,6 +215,11 @@ function registry() { } function mutationCallback() { + // Fire-and-forget: the returned promise is discarded, so route a rejection (a + // constructor throwing while auto-mounting, for instance) to the global error + // handler instead of letting it become an unhandled rejection. In blocking + // mode `addToQueue` runs the task synchronously and returns `undefined` — the + // throw propagates to the mutation observer, hence the optional chaining. addToQueue(() => { for (const [nameOrSelector, ctor] of registry()) { for (const el of getComponentElements(nameOrSelector)) { @@ -189,7 +228,9 @@ function mutationCallback() { } } } - }); + })?.catch((error) => + reportQueuedTaskError(error, 'Auto-mounting components after a DOM mutation failed.'), + ); addToQueue(() => { for (const instance of getInstances()) { @@ -197,7 +238,9 @@ function mutationCallback() { instance.$terminate(); } } - }); + })?.catch((error) => + reportQueuedTaskError(error, 'Auto-terminating components after a DOM mutation failed.'), + ); } export function addToRegistry(nameOrSelector: string, ctor: BaseConstructor) { diff --git a/packages/js-toolkit/helpers/registerComponent.ts b/packages/js-toolkit/helpers/registerComponent.ts index f0a09b18d..0ec9c9df0 100644 --- a/packages/js-toolkit/helpers/registerComponent.ts +++ b/packages/js-toolkit/helpers/registerComponent.ts @@ -11,8 +11,10 @@ import { isDev, isFunction } from '../utils/index.js'; * - a factory function returning such a promise (`() => import(...)`). * * Instances are mounted independently: an element that fails to mount is - * skipped (and logged in development) instead of failing the whole call, so - * the resolved array contains every instance that mounted successfully. + * skipped instead of failing the whole call, so the resolved array contains + * every instance that mounted successfully. Mount failures reach the global + * error handler through `$mount`; a constructor that throws is logged in + * development. * * @link https://js-toolkit.studiometa.dev/api/helpers/registerComponent.html * @param ctor The component constructor, or a way to resolve it. @@ -45,9 +47,16 @@ export async function registerComponent { if (result.status === 'fulfilled') { - return [result.value]; + // `$mount()` resolves with the instance even when a queued mount task + // fails — the error goes to the global handler instead of rejecting, so a + // fire-and-forget mount can not become an unhandled rejection. Check the + // flag rather than the promise state to keep the documented contract: + // only instances that actually mounted are returned. + return result.value.$isMounted ? [result.value] : []; } + // The promise still rejects when the instance could not even be + // constructed, which no other channel reports. if (isDev) { console.error( '[registerComponent] An instance failed to mount and was skipped.', diff --git a/packages/js-toolkit/utils/Queue.ts b/packages/js-toolkit/utils/Queue.ts index 7894c4a61..3a096aadf 100644 --- a/packages/js-toolkit/utils/Queue.ts +++ b/packages/js-toolkit/utils/Queue.ts @@ -40,8 +40,19 @@ export class Queue { * Add a task to the queue. */ add(task: () => unknown) { - const p = new Promise((resolve) => { - this.tasks.push(() => resolve(task())); + const p = new Promise((resolve, reject) => { + this.tasks.push(() => { + try { + resolve(task()); + } catch (err) { + // Containing the throw here is what keeps the queue alive: left to + // escape, it would abort `run()` mid-batch, orphan the remaining + // tasks and leave `isScheduled` stuck `true`, wedging the queue for + // good. Rejecting the returned promise also makes a synchronous throw + // behave exactly like an `async` task whose promise rejects. + reject(err); + } + }); }); this.scheduleFlush(); return p; @@ -56,7 +67,20 @@ export class Queue { } this.isScheduled = true; - this.waiter(() => this.flush()); + + try { + this.waiter(() => this.flush()); + } catch (err) { + // `waiter` is a public constructor parameter, so a faulty scheduler is + // reachable. Reset the flag before re-throwing: stuck `true`, it would + // make every later `scheduleFlush()` early-return and never re-arm a + // flush. The task `add()` just pushed stays queued on purpose — it runs on + // the next flush that does get scheduled, and its promise never reached + // the caller (the throw pre-empts `add()`'s `return`), so rejecting it + // here could only produce an unobservable rejection. + this.isScheduled = false; + throw err; + } } /** diff --git a/packages/tests/Base/Base.spec.ts b/packages/tests/Base/Base.spec.ts index 2e0ca7d51..258e7892d 100644 --- a/packages/tests/Base/Base.spec.ts +++ b/packages/tests/Base/Base.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest'; import { Base, BaseConfig, @@ -8,7 +8,8 @@ import { withExtraConfig, withName, } from '@studiometa/js-toolkit'; -import { h, mount } from '#test-utils'; +import { nextTick } from '@studiometa/js-toolkit/utils'; +import { h, mount, mockFeatures } from '#test-utils'; let mockedIsDev = true; @@ -737,3 +738,241 @@ describe('A Base instance config', () => { process.env.NODE_ENV = 'test'; }); }); + +describe('The Base lifecycle error boundaries', () => { + let reported: Error[]; + let microtask: MockInstance; + let restoreFeatures: (() => void) | undefined; + + beforeEach(() => { + reported = []; + // A failed queued lifecycle task is re-thrown from a microtask so it reaches + // `window.onerror` and error monitors. Intercept that channel here, both to + // assert on it and to keep the re-throw from failing the test run. + microtask = vi.spyOn(globalThis, 'queueMicrotask').mockImplementation((callback) => { + try { + callback(); + } catch (error) { + reported.push(error as Error); + } + }); + }); + + afterEach(() => { + microtask.mockRestore(); + restoreFeatures?.(); + restoreFeatures = undefined; + }); + + /** + * Let the shared queue drain across a few ticks and give any pending + * rejection the chance to surface. + */ + async function flushQueue() { + for (let i = 0; i < 5; i += 1) { + // eslint-disable-next-line no-await-in-loop + await nextTick(); + } + } + + it('should not falsely mark the component mounted when a queued mount task fails', async () => { + class Foo extends Base { + static config: BaseConfig = { + name: 'Foo', + }; + } + + const foo = new Foo(h('div')); + const error = new Error('enable boom'); + // Make a queued wiring task throw synchronously, mimicking e.g. + // `__services.enableAll()` failing during `$mount`. + vi.spyOn(foo.__services, 'enableAll').mockImplementation(() => { + throw error; + }); + + const afterMounted = vi.fn(); + foo.$on('after-mounted', afterMounted); + + // `$mount` resolves (never rejects) so a fire-and-forget mount can not turn + // into an unhandled rejection. + await expect(foo.$mount()).resolves.toBe(foo); + + // The wiring never completed, so the component stays unmounted... + expect(foo.$isMounted).toBe(false); + // ...`after-mounted` is not emitted as if init had completed... + expect(afterMounted).not.toHaveBeenCalled(); + // ...and the failure reaches the global error handler, carrying the + // component identity and the original error as its cause. + expect(reported).toHaveLength(1); + expect(reported[0].message).toBe(`[${foo.$id}] The \`$mount\` lifecycle failed.`); + expect(reported[0].cause).toBe(error); + }); + + it('should keep mounting unrelated components after one component fails to mount', async () => { + class Failing extends Base { + static config: BaseConfig = { + name: 'Failing', + }; + + mounted() { + throw new Error('mounted boom'); + } + } + + class Working extends Base { + static config: BaseConfig = { + name: 'Working', + }; + } + + const failing = new Failing(h('div')); + const working = new Working(h('div')); + + // Fire-and-forget, like `mutationCallback` does: the failing component's + // tasks are queued on the shared module-level queue first. + failing.$mount(); + // The unrelated component is queued right after, so its tasks land in the + // same batch as the failing one. + const p = working.$mount(); + + await expect(p).resolves.toBe(working); + await flushQueue(); + + // The unrelated component mounted normally — one bad component no longer + // wedges the shared queue and orphans every other component on the page. + expect(working.$isMounted).toBe(true); + // And the queue keeps accepting work afterwards. + const late = new Working(h('div')); + await late.$mount(); + expect(late.$isMounted).toBe(true); + + // The failure was reported once, not swallowed. + expect(reported).toHaveLength(1); + expect(reported[0].message).toBe(`[${failing.$id}] The \`$mount\` lifecycle failed.`); + }); + + it('should not emit `after-destroyed` when the `destroyed` hook throws', async () => { + const error = new Error('destroyed boom'); + + class Foo extends Base { + static config: BaseConfig = { + name: 'Foo', + }; + + destroyed() { + throw error; + } + } + + const foo = new Foo(h('div')); + await foo.$mount(); + + const afterDestroyed = vi.fn(); + foo.$on('after-destroyed', afterDestroyed); + + await expect(foo.$destroy()).resolves.toBe(foo); + expect(afterDestroyed).not.toHaveBeenCalled(); + + expect(reported).toHaveLength(1); + expect(reported[0].message).toBe(`[${foo.$id}] The \`$destroy\` lifecycle failed.`); + expect(reported[0].cause).toBe(error); + }); + + it('should resolve and report when the `updated` hook throws', async () => { + const error = new Error('updated boom'); + + class Foo extends Base { + static config: BaseConfig = { + name: 'Foo', + }; + + updated() { + throw error; + } + } + + const foo = new Foo(h('div')); + await foo.$mount(); + + await expect(foo.$update()).resolves.toBe(foo); + + expect(reported).toHaveLength(1); + expect(reported[0].message).toBe(`[${foo.$id}] The \`$update\` lifecycle failed.`); + expect(reported[0].cause).toBe(error); + }); + + it('should resolve and report when the `terminated` hook throws', async () => { + const error = new Error('terminated boom'); + + class Foo extends Base { + static config: BaseConfig = { + name: 'Foo', + }; + + terminated() { + throw error; + } + } + + const foo = new Foo(h('div')); + await foo.$mount(); + + await expect(foo.$terminate()).resolves.toBeUndefined(); + + expect(reported).toHaveLength(1); + expect(reported[0].message).toBe(`[${foo.$id}] The \`$terminate\` lifecycle failed.`); + expect(reported[0].cause).toBe(error); + }); + + it('should not produce an unhandled rejection when a fire-and-forget mount fails', async () => { + class Foo extends Base { + static config: BaseConfig = { + name: 'Foo', + }; + + mounted() { + throw new Error('mounted boom'); + } + } + + const rejections: unknown[] = []; + const onRejection = (reason: unknown) => rejections.push(reason); + process.on('unhandledRejection', onRejection); + + // Fire-and-forget: the returned promise is discarded, like `mutationCallback`. + new Foo(h('div')).$mount(); + + await flushQueue(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + process.off('unhandledRejection', onRejection); + + // No unhandled rejection: the lifecycle promise resolves... + expect(rejections).toHaveLength(0); + // ...but the failure is still visible on the global error channel. + expect(reported).toHaveLength(1); + }); + + it('should keep rejecting in blocking mode', async () => { + restoreFeatures = mockFeatures({ blocking: true }).unmock; + const error = new Error('mounted boom'); + + class Foo extends Base { + static config: BaseConfig = { + name: 'Foo', + }; + + mounted() { + throw error; + } + } + + const foo = new Foo(h('div')); + + // No queue is involved in blocking mode: the task runs in the caller's own + // stack, so `try { await createApp(App, { blocking: true }) } catch` keeps + // working and the error is not deferred to a microtask. + await expect(foo.$mount()).rejects.toBe(error); + expect(reported).toHaveLength(0); + }); +}); diff --git a/packages/tests/helpers/registerComponent.spec.ts b/packages/tests/helpers/registerComponent.spec.ts index ae780ed3e..8e4a7f678 100644 --- a/packages/tests/helpers/registerComponent.spec.ts +++ b/packages/tests/helpers/registerComponent.spec.ts @@ -159,4 +159,39 @@ describe('The `registerComponent` lazy import helper', () => { errorSpy.mockRestore(); }); + + it('should skip an instance whose mount failed', async () => { + const failing = h('div', { dataComponent: 'Component' }); + failing.setAttribute('data-fail', ''); + document.body.append(failing); + + class Component extends Base { + static config = { + name: 'Component', + }; + + constructor(el: HTMLElement) { + super(el); + if (el.hasAttribute('data-fail')) { + // Fail while wiring, so the instance never becomes mounted. + this.__services.enableAll = () => { + throw new Error('boom'); + }; + } + } + } + + // `$mount()` resolves instead of rejecting, so the failure is reported on + // the global error channel. Intercept it here. + const microtask = vi.spyOn(globalThis, 'queueMicrotask').mockImplementation(() => {}); + + // A never-mounted instance must not be handed back as a successful + // registration. + const instances = await registerComponent(Component); + expect(instances).toHaveLength(1); + expect(instances[0].$el).not.toBe(failing); + expect(instances[0].$isMounted).toBe(true); + + microtask.mockRestore(); + }); }); diff --git a/packages/tests/utils/Queue.spec.ts b/packages/tests/utils/Queue.spec.ts index 350231fa2..df4720677 100644 --- a/packages/tests/utils/Queue.spec.ts +++ b/packages/tests/utils/Queue.spec.ts @@ -37,4 +37,114 @@ describe('The `Queue` class', () => { await p; expect(spy).toHaveBeenCalledTimes(1); }); + + it('should keep running the rest of the batch when a task throws', async () => { + // Use a manual waiter so all three tasks land in a single pending batch and + // one flush drains them together — this is what exercises the same-batch + // isolation (an immediate waiter would flush each `add()` on its own). + let scheduled: (() => void) | undefined; + const queue = new Queue(10, (cb) => { + scheduled = cb as () => void; + }); + const before = vi.fn(); + const after = vi.fn(); + const error = new Error('boom'); + + queue.add(before); + // The throwing task's promise rejects; observe it so it is not reported as + // an unhandled rejection. + const throwing = queue.add(() => { + throw error; + }); + throwing.catch(() => {}); + queue.add(after); + + // Nothing ran yet: all three tasks share the same pending batch. + expect(before).not.toHaveBeenCalled(); + + // Trigger the single flush that drains the whole batch. + scheduled?.(); + + // The throwing task must not abort the tasks queued before or after it. + expect(before).toHaveBeenCalledTimes(1); + expect(after).toHaveBeenCalledTimes(1); + + // The failure is surfaced through the task's own promise, which rejects. + await expect(throwing).rejects.toBe(error); + }); + + it('should not wedge the queue for future flushes when a task throws', () => { + const queue = new Queue(10); + + const throwing = queue.add(() => { + throw new Error('boom'); + }); + // Observe the rejection so it does not surface as an unhandled rejection. + throwing.catch(() => {}); + + // The scheduling flag must have been reset by the previous flush. + expect(queue.isScheduled).toBe(false); + + // A subsequent enqueue must still schedule and run. + const spy = vi.fn(); + queue.add(spy); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('should reject consistently for sync and async throws', async () => { + const queue = new Queue(10); + const error = new Error('boom'); + + const sync = queue.add(() => { + throw error; + }); + const async = queue.add(async () => { + throw error; + }); + + await expect(sync).rejects.toBe(error); + await expect(async).rejects.toBe(error); + }); + + it('should reset the scheduling flag when the scheduler throws', () => { + const error = new Error('scheduler boom'); + const queue = new Queue(1, () => { + throw error; + }); + + // The throw propagates to the caller... + expect(() => + queue.add(() => { + /* noop */ + }), + ).toThrow(error); + + // ...but the flag is reset so the queue is not permanently wedged. + expect(queue.isScheduled).toBe(false); + }); + + it('should keep the pending task queued when the scheduler throws', () => { + let broken = true; + const queue = new Queue(10, (cb) => { + if (broken) { + throw new Error('scheduler boom'); + } + (cb as () => void)(); + }); + const spy = vi.fn(); + + expect(() => queue.add(spy)).toThrow('scheduler boom'); + // The task is not dropped: it is still queued, waiting for a flush that + // does get scheduled. Rejecting it here would be unobservable, since the + // throw pre-empted `add()` returning its promise. + expect(queue.tasks).toHaveLength(1); + expect(spy).not.toHaveBeenCalled(); + + // Once the scheduler works again, the orphaned task runs with the next one. + broken = false; + const next = vi.fn(); + queue.add(next); + expect(spy).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/tests/utils/SmartQueue.spec.ts b/packages/tests/utils/SmartQueue.spec.ts index 46571f549..f8d7edf87 100644 --- a/packages/tests/utils/SmartQueue.spec.ts +++ b/packages/tests/utils/SmartQueue.spec.ts @@ -25,4 +25,72 @@ describe('The `SmartQueue` class', () => { await nextTick(); expect(spy).toHaveBeenCalledTimes(4); }); + + it('should keep running the batch when a task throws', async () => { + const queue = new SmartQueue(); + const before = vi.fn(); + const after = vi.fn(); + const error = new Error('boom'); + + queue.add(before); + // The throwing task's promise rejects; observe it so it is not reported as + // an unhandled rejection. + const throwing = queue.add(() => { + throw error; + }); + throwing.catch(() => {}); + queue.add(after); + + await nextTick(); + + // Both the task before and after the throwing one must have run. + expect(before).toHaveBeenCalledTimes(1); + expect(after).toHaveBeenCalledTimes(1); + + // The failure is surfaced through the task's own promise, which rejects. + await expect(throwing).rejects.toBe(error); + }); + + it('should not wedge the queue for future flushes when a task throws', async () => { + const queue = new SmartQueue(); + const spy = vi.fn(); + + const throwing = queue.add(() => { + throw new Error('boom'); + }); + // Observe the rejection so it does not surface as an unhandled rejection. + throwing.catch(() => {}); + await nextTick(); + + // The scheduling flag must have been reset by the previous flush. + expect(queue.isScheduled).toBe(false); + + // A subsequent enqueue must still schedule and run. + queue.add(spy); + await nextTick(); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('should inherit the rejection contract for sync and async throws', async () => { + const queue = new SmartQueue(); + const error = new Error('boom'); + + // `SmartQueue` overrides neither `add()` nor the error isolation: a + // synchronous throw rejects exactly like an `async` task whose promise + // rejects. Observe both rejections right away so the flush below can not + // report them as unhandled. + const sync = queue.add(() => { + throw error; + }); + sync.catch(() => {}); + const async = queue.add(async () => { + throw error; + }); + async.catch(() => {}); + + await nextTick(); + + await expect(sync).rejects.toBe(error); + await expect(async).rejects.toBe(error); + }); });