From 34c2900a647e7ac34c2384a5875e02e8dad96654 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 23:10:07 -0400 Subject: [PATCH] WIP --- .../src/internal/client/error-handling.js | 22 ++----- .../svelte/src/internal/client/runtime.js | 63 +++++++++++-------- .../samples/error-boundary-12/_config.js | 7 +-- .../samples/error-boundary-12/main.svelte | 10 ++- .../samples/error-boundary-13/Child.svelte | 13 +++- .../samples/error-boundary-13/_config.js | 2 +- .../samples/error-boundary-13/main.svelte | 11 +--- .../samples/error-boundary-18/_config.js | 8 +-- .../samples/error-boundary-18/main.svelte | 12 ++-- 9 files changed, 72 insertions(+), 76 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 754267184d..ead5e0646f 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -15,37 +15,25 @@ export function reset_is_throwing_error() { is_throwing_error = false; } +/** @type {null | unknown} */ +let current_error = null; + /** * @param {unknown} error * @param {Effect} effect * @param {Effect | null} [previous_effect] */ export function handle_error(error, effect, previous_effect = null) { - if (is_throwing_error) { - if (previous_effect === null) { - is_throwing_error = false; - } - - if (should_rethrow_error(effect)) { - throw error; - } - + if (error === current_error) { + // TODO is this necessary? return; } - if (previous_effect !== null) { - is_throwing_error = true; - } - if (DEV && error instanceof Error) { adjust_error(error, effect); } invoke_error_boundary(error, effect); - - if (should_rethrow_error(effect)) { - throw error; - } } /** diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2ac3125a0f..4eddd7d3bf 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -343,6 +343,13 @@ export function update_reaction(reaction) { } return result; + } catch (error) { + var effect = get_effect(reaction); + if (effect) { + handle_error(error, effect); + } else { + throw error; + } } finally { new_deps = previous_deps; skipped_deps = previous_skipped_deps; @@ -357,6 +364,18 @@ export function update_reaction(reaction) { } } +/** @param {Reaction} reaction */ +function get_effect(reaction) { + /** @type {Reaction | null;} */ + var r = reaction; + + while (r !== null && (r.f & DERIVED) !== 0) { + r = /** @type {Derived} */ (r).parent; + } + + return /** @type {Effect | null} */ (r); +} + /** * @template V * @param {Reaction} signal @@ -470,8 +489,6 @@ export function update_effect(effect) { if (DEV) { dev_effect_stack.push(effect); } - } catch (error) { - handle_error(error, effect, previous_effect); } finally { is_updating_effect = was_updating_effect; active_effect = previous_effect; @@ -570,27 +587,23 @@ function flush_queued_effects(effects) { var effect = effects[i]; if ((effect.f & (DESTROYED | INERT)) === 0) { - try { - if (check_dirtiness(effect)) { - update_effect(effect); - - // Effects with no dependencies or teardown do not get added to the effect tree. - // Deferred effects (e.g. `$effect(...)`) _are_ added to the tree because we - // don't know if we need to keep them until they are executed. Doing the check - // here (rather than in `update_effect`) allows us to skip the work for - // immediate effects. - if (effect.deps === null && effect.first === null && effect.nodes_start === null) { - if (effect.teardown === null) { - // remove this effect from the graph - unlink_effect(effect); - } else { - // keep the effect in the graph, but free up some memory - effect.fn = null; - } + if (check_dirtiness(effect)) { + update_effect(effect); + + // Effects with no dependencies or teardown do not get added to the effect tree. + // Deferred effects (e.g. `$effect(...)`) _are_ added to the tree because we + // don't know if we need to keep them until they are executed. Doing the check + // here (rather than in `update_effect`) allows us to skip the work for + // immediate effects. + if (effect.deps === null && effect.first === null && effect.nodes_start === null) { + if (effect.teardown === null) { + // remove this effect from the graph + unlink_effect(effect); + } else { + // keep the effect in the graph, but free up some memory + effect.fn = null; } } - } catch (error) { - handle_error(error, effect); } } } @@ -649,12 +662,8 @@ function process_effects(root) { } else if (is_branch) { effect.f ^= CLEAN; } else { - try { - if (check_dirtiness(effect)) { - update_effect(effect); - } - } catch (error) { - handle_error(error, effect); + if (check_dirtiness(effect)) { + update_effect(effect); } } diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-12/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-12/_config.js index b3edfefa20..4e771dfafa 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-12/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-12/_config.js @@ -5,9 +5,8 @@ export default test({ test({ assert, target }) { const btn = target.querySelector('button'); - btn?.click(); - flushSync(); - - assert.htmlEqual(target.innerHTML, `

Error occured

`); + assert.throws(() => { + flushSync(() => btn?.click()); + }, /kaboom/); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-12/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-12/main.svelte index bae46c2590..d9dee1e2b0 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-12/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-12/main.svelte @@ -1,20 +1,18 @@ - {#each things as thing} -

{thing}

- {/each} + {d} {#snippet failed()}

Error occured

diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-13/Child.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-13/Child.svelte index 6e607871d3..de482da985 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-13/Child.svelte +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-13/Child.svelte @@ -1,7 +1,14 @@ diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-13/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-13/_config.js index b3edfefa20..3825dc7811 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-13/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-13/_config.js @@ -8,6 +8,6 @@ export default test({ btn?.click(); flushSync(); - assert.htmlEqual(target.innerHTML, `

Error occured

`); + assert.htmlEqual(target.innerHTML, `

Error occurred

`); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-13/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-13/main.svelte index 65b71106fa..56e4846675 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-13/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-13/main.svelte @@ -2,21 +2,14 @@ import Child from './Child.svelte'; let count = $state(0); - - const things = $derived.by(() => { - if (count === 1) { - throw new Error('123') - } - return [1, 2 ,3] - }) - + {#snippet failed()} -

Error occured

+

Error occurred

{/snippet}
diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-18/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-18/_config.js index fedaaa9ad1..e092d0e7c7 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-18/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-18/_config.js @@ -5,11 +5,11 @@ export default test({ test({ assert, target }) { let btn = target.querySelector('button'); - btn?.click(); - btn?.click(); - assert.throws(() => { - flushSync(); + flushSync(() => { + btn?.click(); + btn?.click(); + }); }, /test\n\n\tin {expression}\n/); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-18/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-18/main.svelte index b9de7126db..e73dcacc8a 100644 --- a/packages/svelte/tests/runtime-runes/samples/error-boundary-18/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-18/main.svelte @@ -1,16 +1,18 @@ { throw(e) }}> -
Count: {count}
- - {count} / {test} +
Count: {count}
+ + {count} / {maybe_throw()}