fix: route \$derived teardown errors through invoke_error_boundary

Two bugs fixed:

1. invoke_error_boundary only guarded the initial effect against DESTROYED,
   not ancestors walked in the loop. A fully-destroyed boundary has effect.b
   nulled, causing "Cannot read properties of null (reading 'error')" which
   masks the real error. Fix: move the guard inside the loop and also skip
   DESTROYING effects (mid-teardown).

2. execute_effect_teardown let teardown errors escape raw up the call stack,
   bypassing invoke_error_boundary entirely so <svelte:boundary onerror>
   was never called. Fix: catch and route through invoke_error_boundary.

Fixes #18485

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pull/18486/head
Rupankar Dutta 2 weeks ago
parent eae50dfd1c
commit 62a5df5838

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

@ -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 <svelte:boundary> 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);

@ -0,0 +1,12 @@
<script>
let { getValue } = $props();
// Reads reactive state during teardown, like bits-ui popovers/tooltips do.
$effect(() => {
return () => {
getValue();
};
});
</script>
<span>trigger</span>

@ -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: '<button id="break">break</button><button id="unmount">unmount</button><span>trigger</span>',
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;
}
});

@ -0,0 +1,20 @@
<script>
import Trigger from './Trigger.svelte';
let mounted = $state(true);
let broken = $state(false);
// Throwing derived — nothing in the live template reads `appContext`, so
// invalidating it does NOT throw immediately (becomes dirty-and-unread).
let data = $derived(broken ? null : { value: 'ok' });
let appContext = $derived(data.value);
</script>
<button id="break" onclick={() => (broken = true)}>break</button>
<button id="unmount" onclick={() => (mounted = false)}>unmount</button>
{#if mounted}
<svelte:boundary onerror={(e) => errors.push(e.message)}>
<Trigger getValue={() => appContext} />
</svelte:boundary>
{/if}
Loading…
Cancel
Save