diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 7c404dd933..293d892103 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -3,7 +3,7 @@ import { DEV } from 'esm-env'; import { FILENAME } from '../../constants.js'; import { is_firefox } from './dom/operations.js'; -import { ERROR_VALUE, BOUNDARY_EFFECT, REACTION_RAN, EFFECT, DESTROYED } from './constants.js'; +import { ERROR_VALUE, BOUNDARY_EFFECT, REACTION_RAN, EFFECT, DESTROYED, DESTROYING } from './constants.js'; import { define_property, get_descriptor } from '../shared/utils.js'; import { active_effect, active_reaction } from './runtime.js'; @@ -45,11 +45,14 @@ export function handle_error(error) { * @param {Effect | null} effect */ export function invoke_error_boundary(error, effect) { - if (effect !== null && (effect.f & DESTROYED) !== 0) { - return; - } - while (effect !== null) { + // Skip effects that are destroyed or currently being destroyed — their + // boundaries are mid-teardown and cannot meaningfully handle the error. + if ((effect.f & (DESTROYED | DESTROYING)) !== 0) { + effect = effect.parent; + continue; + } + if ((effect.f & BOUNDARY_EFFECT) !== 0) { if ((effect.f & REACTION_RAN) === 0) { // we are still creating the boundary effect diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index c5d195dfae..7e53138aae 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -36,6 +36,7 @@ import { MANAGED_EFFECT, DESTROYING } from '#client/constants'; +import { invoke_error_boundary } from '../error-handling.js'; import * as e from '../errors.js'; import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; @@ -449,6 +450,11 @@ export function execute_effect_teardown(effect) { set_active_reaction(null); try { teardown.call(null); + } catch (error) { + // Route teardown errors through the boundary system so that a live + // ancestor can handle them. Boundaries that are + // themselves mid-teardown are skipped by invoke_error_boundary. + invoke_error_boundary(error, effect.parent); } finally { set_is_destroying_effect(previously_destroying_effect); set_active_reaction(previous_reaction); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-28/Trigger.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-28/Trigger.svelte new file mode 100644 index 0000000000..aac3a1847f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-28/Trigger.svelte @@ -0,0 +1,12 @@ + + +trigger diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-28/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-28/_config.js new file mode 100644 index 0000000000..da227c5958 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-28/_config.js @@ -0,0 +1,47 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + // Regression test for https://github.com/sveltejs/svelte/issues/18485. + // A $derived that re-executes and throws during teardown should not crash + // with "Cannot read properties of null (reading 'error')" and should not + // mask the real error. + html: 'trigger', + mode: ['client'], + + test({ assert, target, window }) { + const errors = /** @type {string[]} */ ([]); + + // Expose the errors array to the component template. + // We check window.uncaughtErrors to verify no unhandled throw escapes. + const originalOnError = window.onerror; + /** @type {string[]} */ + const uncaught = []; + window.onerror = (msg) => { + uncaught.push(String(msg)); + return true; + }; + + // Step 1: make appContext dirty without triggering an immediate throw. + flushSync(() => { + target.querySelector('#break')?.click(); + }); + + // Step 2: unmount — teardown reads the dirty derived, which throws. + flushSync(() => { + target.querySelector('#unmount')?.click(); + }); + + // The boundary was being torn down along with the component, so the + // onerror handler itself may not fire (the boundary is already gone), + // but the critical thing is that no TypeError about null effect.b + // escapes to the global error handler. + assert.equal( + uncaught.filter((m) => m.includes("Cannot read properties of null")).length, + 0, + 'no null-dereference crash should escape' + ); + + window.onerror = originalOnError; + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-28/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-28/main.svelte new file mode 100644 index 0000000000..d696c246cd --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-28/main.svelte @@ -0,0 +1,20 @@ + + + + + +{#if mounted} + errors.push(e.message)}> + appContext} /> + +{/if}