From fd2e7a28132825bac4131781fa445cf577bbe658 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 18:22:33 -0400 Subject: [PATCH 01/29] remove unused handle_error arguments --- .../src/internal/client/dom/blocks/boundary.js | 2 +- packages/svelte/src/internal/client/runtime.js | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 53060017b935..2e94a8680642 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -109,7 +109,7 @@ export function boundary(node, props, boundary_fn) { ); }); } catch (error) { - handle_error(error, boundary, null, boundary.ctx); + handle_error(error, boundary); } reset_is_throwing_error(); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index d790c0ad145a..2324c725931e 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -271,10 +271,11 @@ export function reset_is_throwing_error() { /** * @param {unknown} error * @param {Effect} effect - * @param {Effect | null} previous_effect - * @param {ComponentContext | null} component_context + * @param {Effect | null} [previous_effect] */ -export function handle_error(error, effect, previous_effect, component_context) { +export function handle_error(error, effect, previous_effect = null) { + var component_context = effect.ctx; + if (is_throwing_error) { if (previous_effect === null) { is_throwing_error = false; @@ -558,7 +559,6 @@ export function update_effect(effect) { set_signal_status(effect, CLEAN); var previous_effect = active_effect; - var previous_component_context = component_context; var was_updating_effect = is_updating_effect; active_effect = effect; @@ -602,7 +602,7 @@ export function update_effect(effect) { dev_effect_stack.push(effect); } } catch (error) { - handle_error(error, effect, previous_effect, previous_component_context || effect.ctx); + handle_error(error, effect, previous_effect); } finally { is_updating_effect = was_updating_effect; active_effect = previous_effect; @@ -637,14 +637,14 @@ function infinite_loop_guard() { if (last_scheduled_effect !== null) { if (DEV) { try { - handle_error(error, last_scheduled_effect, null, null); + handle_error(error, last_scheduled_effect); } catch (e) { // Only log the effect stack if the error is re-thrown log_effect_stack(); throw e; } } else { - handle_error(error, last_scheduled_effect, null, null); + handle_error(error, last_scheduled_effect); } } else { if (DEV) { @@ -721,7 +721,7 @@ function flush_queued_effects(effects) { } } } catch (error) { - handle_error(error, effect, null, effect.ctx); + handle_error(error, effect); } } } @@ -785,7 +785,7 @@ function process_effects(root) { update_effect(effect); } } catch (error) { - handle_error(error, effect, null, effect.ctx); + handle_error(error, effect); } } From c0cdc5a6a32b1c5a1ff48a555ee153a0511c1454 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 18:38:13 -0400 Subject: [PATCH 02/29] move error handling code into separate module --- .../internal/client/dom/blocks/boundary.js | 5 +- .../src/internal/client/error-handling.js | 138 ++++++++++++++++++ .../svelte/src/internal/client/runtime.js | 135 +---------------- 3 files changed, 142 insertions(+), 136 deletions(-) create mode 100644 packages/svelte/src/internal/client/error-handling.js diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 2e94a8680642..0eb00bf630a7 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -2,14 +2,13 @@ import { BOUNDARY_EFFECT, EFFECT_TRANSPARENT } from '#client/constants'; import { component_context, set_component_context } from '../../context.js'; +import { handle_error, reset_is_throwing_error } from '../../error-handling.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; import { active_effect, active_reaction, - handle_error, set_active_effect, - set_active_reaction, - reset_is_throwing_error + set_active_reaction } from '../../runtime.js'; import { hydrate_next, diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js new file mode 100644 index 000000000000..3b59481b0e61 --- /dev/null +++ b/packages/svelte/src/internal/client/error-handling.js @@ -0,0 +1,138 @@ +/** @import { ComponentContext, Effect } from '#client' */ +import { DEV } from 'esm-env'; +import { FILENAME } from '../../constants.js'; +import { is_firefox } from './dom/operations.js'; +import { BOUNDARY_EFFECT, DESTROYED } from './constants.js'; +import { define_property } from '../shared/utils.js'; + +// Used for DEV time error handling +/** @param {WeakSet} value */ +const handled_errors = new WeakSet(); + +let is_throwing_error = false; + +/** + * @param {unknown} error + * @param {Effect} effect + * @param {Effect | null} [previous_effect] + */ +export function handle_error(error, effect, previous_effect = null) { + var component_context = effect.ctx; + + if (is_throwing_error) { + if (previous_effect === null) { + is_throwing_error = false; + } + + if (should_rethrow_error(effect)) { + throw error; + } + + return; + } + + if (previous_effect !== null) { + is_throwing_error = true; + } + + if (DEV && component_context !== null && error instanceof Error && !handled_errors.has(error)) { + handled_errors.add(error); + + const component_stack = []; + + const effect_name = effect.fn?.name; + + if (effect_name) { + component_stack.push(effect_name); + } + + /** @type {ComponentContext | null} */ + let current_context = component_context; + + while (current_context !== null) { + /** @type {string} */ + var filename = current_context.function?.[FILENAME]; + + if (filename) { + const file = filename.split('/').pop(); + component_stack.push(file); + } + + current_context = current_context.p; + } + + const indent = is_firefox ? ' ' : '\t'; + define_property(error, 'message', { + value: + error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` + }); + define_property(error, 'component_stack', { + value: component_stack + }); + + const stack = error.stack; + + // Filter out internal files from callstack + if (stack) { + const lines = stack.split('\n'); + const new_lines = []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (line.includes('svelte/src/internal')) { + continue; + } + new_lines.push(line); + } + define_property(error, 'stack', { + value: new_lines.join('\n') + }); + } + } + + propagate_error(error, effect); + + if (should_rethrow_error(effect)) { + throw error; + } +} + +/** + * @param {unknown} error + * @param {Effect} effect + */ +function propagate_error(error, effect) { + /** @type {Effect | null} */ + var current = effect; + + while (current !== null) { + if ((current.f & BOUNDARY_EFFECT) !== 0) { + try { + // @ts-expect-error + current.fn(error); + return; + } catch { + // Remove boundary flag from effect + current.f ^= BOUNDARY_EFFECT; + } + } + + current = current.parent; + } + + is_throwing_error = false; + throw error; +} + +/** + * @param {Effect} effect + */ +function should_rethrow_error(effect) { + return ( + (effect.f & DESTROYED) === 0 && + (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) + ); +} + +export function reset_is_throwing_error() { + is_throwing_error = false; +} diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2324c725931e..2ac3125a0f76 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -29,7 +29,7 @@ import { flush_tasks } from './dom/task.js'; import { internal_set, old_values } from './reactivity/sources.js'; import { destroy_derived_effects, update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; -import { FILENAME } from '../../constants.js'; + import { tracing_mode_flag } from '../flags/index.js'; import { tracing_expressions, get_stack } from './dev/tracing.js'; import { @@ -39,12 +39,7 @@ import { set_component_context, set_dev_current_component_function } from './context.js'; -import { is_firefox } from './dom/operations.js'; - -// Used for DEV time error handling -/** @param {WeakSet} value */ -const handled_errors = new WeakSet(); -let is_throwing_error = false; +import { handle_error } from './error-handling.js'; let is_flushing = false; @@ -227,132 +222,6 @@ export function check_dirtiness(reaction) { return false; } -/** - * @param {unknown} error - * @param {Effect} effect - */ -function propagate_error(error, effect) { - /** @type {Effect | null} */ - var current = effect; - - while (current !== null) { - if ((current.f & BOUNDARY_EFFECT) !== 0) { - try { - // @ts-expect-error - current.fn(error); - return; - } catch { - // Remove boundary flag from effect - current.f ^= BOUNDARY_EFFECT; - } - } - - current = current.parent; - } - - is_throwing_error = false; - throw error; -} - -/** - * @param {Effect} effect - */ -function should_rethrow_error(effect) { - return ( - (effect.f & DESTROYED) === 0 && - (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) - ); -} - -export function reset_is_throwing_error() { - is_throwing_error = false; -} - -/** - * @param {unknown} error - * @param {Effect} effect - * @param {Effect | null} [previous_effect] - */ -export function handle_error(error, effect, previous_effect = null) { - var component_context = effect.ctx; - - if (is_throwing_error) { - if (previous_effect === null) { - is_throwing_error = false; - } - - if (should_rethrow_error(effect)) { - throw error; - } - - return; - } - - if (previous_effect !== null) { - is_throwing_error = true; - } - - if (DEV && component_context !== null && error instanceof Error && !handled_errors.has(error)) { - handled_errors.add(error); - - const component_stack = []; - - const effect_name = effect.fn?.name; - - if (effect_name) { - component_stack.push(effect_name); - } - - /** @type {ComponentContext | null} */ - let current_context = component_context; - - while (current_context !== null) { - /** @type {string} */ - var filename = current_context.function?.[FILENAME]; - - if (filename) { - const file = filename.split('/').pop(); - component_stack.push(file); - } - - current_context = current_context.p; - } - - const indent = is_firefox ? ' ' : '\t'; - define_property(error, 'message', { - value: - error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` - }); - define_property(error, 'component_stack', { - value: component_stack - }); - - const stack = error.stack; - - // Filter out internal files from callstack - if (stack) { - const lines = stack.split('\n'); - const new_lines = []; - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - if (line.includes('svelte/src/internal')) { - continue; - } - new_lines.push(line); - } - define_property(error, 'stack', { - value: new_lines.join('\n') - }); - } - } - - propagate_error(error, effect); - - if (should_rethrow_error(effect)) { - throw error; - } -} - /** * @param {Value} signal * @param {Effect} effect From 04817caf83dc848797f6473486105d576827a7ee Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 18:41:07 -0400 Subject: [PATCH 03/29] tidy up --- .../svelte/src/internal/client/error-handling.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 3b59481b0e61..90294ba5cef2 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -11,6 +11,10 @@ const handled_errors = new WeakSet(); let is_throwing_error = false; +export function reset_is_throwing_error() { + is_throwing_error = false; +} + /** * @param {unknown} error * @param {Effect} effect @@ -89,7 +93,7 @@ export function handle_error(error, effect, previous_effect = null) { } } - propagate_error(error, effect); + invoke_error_boundary(error, effect); if (should_rethrow_error(effect)) { throw error; @@ -100,7 +104,7 @@ export function handle_error(error, effect, previous_effect = null) { * @param {unknown} error * @param {Effect} effect */ -function propagate_error(error, effect) { +function invoke_error_boundary(error, effect) { /** @type {Effect | null} */ var current = effect; @@ -132,7 +136,3 @@ function should_rethrow_error(effect) { (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) ); } - -export function reset_is_throwing_error() { - is_throwing_error = false; -} From f077415e43dfa41f36e62901b6717f43a0d73fac Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 18:52:36 -0400 Subject: [PATCH 04/29] simplify --- .../src/internal/client/error-handling.js | 99 ++++++++----------- 1 file changed, 43 insertions(+), 56 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 90294ba5cef2..754267184d4c 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -1,4 +1,4 @@ -/** @import { ComponentContext, Effect } from '#client' */ +/** @import { Effect } from '#client' */ import { DEV } from 'esm-env'; import { FILENAME } from '../../constants.js'; import { is_firefox } from './dom/operations.js'; @@ -7,7 +7,7 @@ import { define_property } from '../shared/utils.js'; // Used for DEV time error handling /** @param {WeakSet} value */ -const handled_errors = new WeakSet(); +const adjusted_errors = new WeakSet(); let is_throwing_error = false; @@ -21,8 +21,6 @@ export function reset_is_throwing_error() { * @param {Effect | null} [previous_effect] */ export function handle_error(error, effect, previous_effect = null) { - var component_context = effect.ctx; - if (is_throwing_error) { if (previous_effect === null) { is_throwing_error = false; @@ -39,58 +37,8 @@ export function handle_error(error, effect, previous_effect = null) { is_throwing_error = true; } - if (DEV && component_context !== null && error instanceof Error && !handled_errors.has(error)) { - handled_errors.add(error); - - const component_stack = []; - - const effect_name = effect.fn?.name; - - if (effect_name) { - component_stack.push(effect_name); - } - - /** @type {ComponentContext | null} */ - let current_context = component_context; - - while (current_context !== null) { - /** @type {string} */ - var filename = current_context.function?.[FILENAME]; - - if (filename) { - const file = filename.split('/').pop(); - component_stack.push(file); - } - - current_context = current_context.p; - } - - const indent = is_firefox ? ' ' : '\t'; - define_property(error, 'message', { - value: - error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` - }); - define_property(error, 'component_stack', { - value: component_stack - }); - - const stack = error.stack; - - // Filter out internal files from callstack - if (stack) { - const lines = stack.split('\n'); - const new_lines = []; - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - if (line.includes('svelte/src/internal')) { - continue; - } - new_lines.push(line); - } - define_property(error, 'stack', { - value: new_lines.join('\n') - }); - } + if (DEV && error instanceof Error) { + adjust_error(error, effect); } invoke_error_boundary(error, effect); @@ -136,3 +84,42 @@ function should_rethrow_error(effect) { (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) ); } + +/** + * Add useful information to the error message/stack + * @param {Error} error + * @param {Effect} effect + */ +function adjust_error(error, effect) { + if (adjusted_errors.has(error)) return; + adjusted_errors.add(error); + + const component_stack = [effect.fn?.name ?? '']; + const indent = is_firefox ? ' ' : '\t'; + + var context = effect.ctx; + + while (context !== null) { + component_stack.push(context.function?.[FILENAME].split('/').pop()); + context = context.p; + } + + define_property(error, 'message', { + value: error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` + }); + + // TODO what is this for? can we get rid of it? + define_property(error, 'component_stack', { + value: component_stack + }); + + // Filter out internal files from callstack + if (error.stack) { + define_property(error, 'stack', { + value: error.stack + .split('\n') + .filter((line) => !line.includes('svelte/src/internal')) + .join('\n') + }); + } +} From 7d72187fedc04aa75bc340f71831624b3186cd8b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 19:25:26 -0400 Subject: [PATCH 05/29] remove unnecessary handle_error invocation --- packages/svelte/src/internal/client/dom/blocks/boundary.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 0eb00bf630a7..064c5212185a 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -2,7 +2,7 @@ import { BOUNDARY_EFFECT, EFFECT_TRANSPARENT } from '#client/constants'; import { component_context, set_component_context } from '../../context.js'; -import { handle_error, reset_is_throwing_error } from '../../error-handling.js'; +import { reset_is_throwing_error } from '../../error-handling.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; import { active_effect, @@ -107,8 +107,8 @@ export function boundary(node, props, boundary_fn) { () => reset ); }); - } catch (error) { - handle_error(error, boundary); + } catch { + // do nothing, handle_error has already been invoked } reset_is_throwing_error(); From 34c2900a647e7ac34c2384a5875e02e8dad96654 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 23:10:07 -0400 Subject: [PATCH 06/29] 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 754267184d4c..ead5e0646fc5 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 2ac3125a0f76..4eddd7d3bfff 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 b3edfefa2002..4e771dfafa04 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 bae46c2590dc..d9dee1e2b037 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 6e607871d38f..de482da985e9 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 b3edfefa2002..3825dc781194 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 65b71106fab9..56e484667597 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 fedaaa9ad11f..e092d0e7c752 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 b9de7126dbf7..e73dcacc8aa7 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()}
From 1d37bf1fb61926c3b7c01eada4ea06c92d3e1fc7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Jun 2025 23:34:31 -0400 Subject: [PATCH 07/29] WIP --- .../svelte/src/internal/client/dom/blocks/boundary.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 064c5212185a..a886c7d7919a 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -83,7 +83,14 @@ export function boundary(node, props, boundary_fn) { }); }; - onerror?.(error, reset); + var previous_reaction = active_reaction; + + try { + set_active_reaction(null); + onerror?.(error, reset); + } finally { + set_active_reaction(previous_reaction); + } if (boundary_effect) { destroy_effect(boundary_effect); From 051e0e3ebe74a409d4656f6352ee5c2104125754 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 06:42:57 -0400 Subject: [PATCH 08/29] WIP --- .../src/internal/client/reactivity/effects.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 36be1ecd0427..ca1da7f9cd79 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -332,16 +332,23 @@ export function render_effect(fn) { * @returns {Effect} */ export function template_effect(fn, thunks = [], d = derived) { - const deriveds = thunks.map(d); - const effect = () => fn(...deriveds.map(get)); - if (DEV) { - define_property(effect, 'name', { - value: '{expression}' + // wrap the effect so that we can decorate stack trace with `in {expression}` + // (TODO maybe there's a better approach?) + return render_effect(() => { + var outer = /** @type {Effect} */ (active_effect); + var inner = () => fn(...deriveds.map(get)); + + define_property(outer.fn, 'name', { value: '{expression}' }); + define_property(inner, 'name', { value: '{expression}' }); + + const deriveds = thunks.map(d); + block(inner); }); } - return block(effect); + const deriveds = thunks.map(d); + return block(() => fn(...deriveds.map(get))); } /** From 682490dfd9c7923f2632ae1938b8185ab4294ae1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 06:43:16 -0400 Subject: [PATCH 09/29] note to self --- packages/svelte/src/internal/client/dom/blocks/boundary.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index a886c7d7919a..634fed0607cd 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -74,6 +74,7 @@ export function boundary(node, props, boundary_fn) { } var reset = () => { + // TODO does this make sense? pause_effect(boundary_effect); with_boundary(boundary, () => { From dc7e21d18e7126b517b361f8d505f43e4921db5d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 08:05:43 -0400 Subject: [PATCH 10/29] WIP --- .../svelte/src/internal/client/error-handling.js | 2 +- .../src/internal/client/reactivity/effects.js | 9 ++------- packages/svelte/src/internal/client/runtime.js | 16 +++++++++++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index ead5e0646fc5..eaca6cf390d5 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -40,7 +40,7 @@ export function handle_error(error, effect, previous_effect = null) { * @param {unknown} error * @param {Effect} effect */ -function invoke_error_boundary(error, effect) { +export function invoke_error_boundary(error, effect) { /** @type {Effect | null} */ var current = effect; diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index ca1da7f9cd79..6c7c21d7dc88 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -114,13 +114,8 @@ function create_effect(type, fn, sync, push = true) { } if (sync) { - try { - update_effect(effect); - effect.f |= EFFECT_RAN; - } catch (e) { - destroy_effect(effect); - throw e; - } + update_effect(effect); + effect.f |= EFFECT_RAN; } else if (fn !== null) { schedule_effect(effect); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 4eddd7d3bfff..7745f975f851 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -23,7 +23,8 @@ import { LEGACY_DERIVED_PROP, DISCONNECTED, BOUNDARY_EFFECT, - EFFECT_IS_UPDATING + EFFECT_IS_UPDATING, + EFFECT_RAN } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { internal_set, old_values } from './reactivity/sources.js'; @@ -39,7 +40,7 @@ import { set_component_context, set_dev_current_component_function } from './context.js'; -import { handle_error } from './error-handling.js'; +import { handle_error, invoke_error_boundary } from './error-handling.js'; let is_flushing = false; @@ -344,9 +345,18 @@ export function update_reaction(reaction) { return result; } catch (error) { + // TODO think we can just use active_effect here? var effect = get_effect(reaction); + if (effect) { - handle_error(error, effect); + if ((effect.f & EFFECT_RAN) !== 0) { + invoke_error_boundary(error, effect); + } else if ((effect.f & BOUNDARY_EFFECT) !== 0) { + // invoke directly + effect.fn(error); + } else { + throw error; + } } else { throw error; } From dcbd613e3098cea0dd41511e3da00992085bf90e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 08:16:39 -0400 Subject: [PATCH 11/29] WIP --- packages/svelte/src/internal/client/dom/blocks/boundary.js | 6 +++--- packages/svelte/src/internal/client/error-handling.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 634fed0607cd..2387b22f5198 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -2,7 +2,7 @@ import { BOUNDARY_EFFECT, EFFECT_TRANSPARENT } from '#client/constants'; import { component_context, set_component_context } from '../../context.js'; -import { reset_is_throwing_error } from '../../error-handling.js'; +import { invoke_error_boundary, reset_is_throwing_error } from '../../error-handling.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; import { active_effect, @@ -115,8 +115,8 @@ export function boundary(node, props, boundary_fn) { () => reset ); }); - } catch { - // do nothing, handle_error has already been invoked + } catch (error) { + invoke_error_boundary(error, /** @type {Effect} */ (boundary.parent)); } reset_is_throwing_error(); diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index eaca6cf390d5..19f6cba2b88c 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -51,7 +51,7 @@ export function invoke_error_boundary(error, effect) { current.fn(error); return; } catch { - // Remove boundary flag from effect + // Remove boundary flag from effect (TODO is this still useful?) current.f ^= BOUNDARY_EFFECT; } } From 4c33df256e4b28b7aa8603724e4d9912c40b4c8d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 08:25:41 -0400 Subject: [PATCH 12/29] unused --- packages/svelte/src/internal/client/error-handling.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 19f6cba2b88c..410b1e4a80d6 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -21,9 +21,8 @@ let current_error = null; /** * @param {unknown} error * @param {Effect} effect - * @param {Effect | null} [previous_effect] */ -export function handle_error(error, effect, previous_effect = null) { +export function handle_error(error, effect) { if (error === current_error) { // TODO is this necessary? return; From 5256e491eb8c0353c664989c1b1b51e057723352 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 08:31:22 -0400 Subject: [PATCH 13/29] WIP --- .../src/internal/client/error-handling.js | 2 +- .../svelte/src/internal/client/runtime.js | 25 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 410b1e4a80d6..c462b207cecf 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -77,7 +77,7 @@ function should_rethrow_error(effect) { * @param {Error} error * @param {Effect} effect */ -function adjust_error(error, effect) { +export function adjust_error(error, effect) { if (adjusted_errors.has(error)) return; adjusted_errors.add(error); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 7745f975f851..0199f2206923 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -40,7 +40,7 @@ import { set_component_context, set_dev_current_component_function } from './context.js'; -import { handle_error, invoke_error_boundary } from './error-handling.js'; +import { adjust_error, handle_error, invoke_error_boundary } from './error-handling.js'; let is_flushing = false; @@ -345,18 +345,17 @@ export function update_reaction(reaction) { return result; } catch (error) { - // TODO think we can just use active_effect here? - var effect = get_effect(reaction); - - if (effect) { - if ((effect.f & EFFECT_RAN) !== 0) { - invoke_error_boundary(error, effect); - } else if ((effect.f & BOUNDARY_EFFECT) !== 0) { - // invoke directly - effect.fn(error); - } else { - throw error; - } + var effect = /** @type {Effect} */ (active_effect); + + if (DEV && error instanceof Error) { + adjust_error(error, effect); + } + + if ((effect.f & EFFECT_RAN) !== 0) { + invoke_error_boundary(error, effect); + } else if ((effect.f & BOUNDARY_EFFECT) !== 0) { + // invoke directly + effect.fn(error); } else { throw error; } From 4b0ec078560c8f51654ef5b4226da4acda659d44 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 09:59:53 -0400 Subject: [PATCH 14/29] turns out we need this --- .../svelte/src/internal/client/reactivity/effects.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 6c7c21d7dc88..ca1da7f9cd79 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -114,8 +114,13 @@ function create_effect(type, fn, sync, push = true) { } if (sync) { - update_effect(effect); - effect.f |= EFFECT_RAN; + try { + update_effect(effect); + effect.f |= EFFECT_RAN; + } catch (e) { + destroy_effect(effect); + throw e; + } } else if (fn !== null) { schedule_effect(effect); } From 9b8439a92d53a7a56cb456d6e56f8f5afb72432f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 10:12:52 -0400 Subject: [PATCH 15/29] fix --- packages/svelte/src/internal/client/reactivity/effects.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index ca1da7f9cd79..54994b9bd1cd 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -433,7 +433,11 @@ export function destroy_block_effect_children(signal) { export function destroy_effect(effect, remove_dom = true) { var removed = false; - if ((remove_dom || (effect.f & HEAD_EFFECT) !== 0) && effect.nodes_start !== null) { + if ( + (remove_dom || (effect.f & HEAD_EFFECT) !== 0) && + effect.nodes_start !== null && + effect.nodes_end !== null + ) { remove_effect_dom(effect.nodes_start, /** @type {TemplateNode} */ (effect.nodes_end)); removed = true; } From c4f528a7bff3f9481b8ef4147e1510af1b5f9b0f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:16:54 -0400 Subject: [PATCH 16/29] all tests passing --- .../client/visitors/SvelteBoundary.js | 81 +++++++++---------- .../src/internal/client/error-handling.js | 2 +- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js index ec1c65081496..1480648ce5cc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Statement, Expression } from 'estree' */ +/** @import { BlockStatement, Statement, Expression, FunctionDeclaration, VariableDeclaration, ArrowFunctionExpression } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import { dev } from '../../../../state.js'; @@ -34,62 +34,61 @@ export function SvelteBoundary(node, context) { const nodes = []; /** @type {Statement[]} */ - const external_statements = []; + const const_tags = []; /** @type {Statement[]} */ - const internal_statements = []; + const hoisted = []; - const snippets_visits = []; + // const tags need to live inside the boundary, but might also be referenced in hoisted snippets. + // to resolve this we cheat: we duplicate const tags inside snippets + for (const child of node.fragment.nodes) { + if (child.type === 'ConstTag') { + context.visit(child, { ...context.state, init: const_tags }); + } + } - // Capture the `failed` implicit snippet prop for (const child of node.fragment.nodes) { - if (child.type === 'SnippetBlock' && child.expression.name === 'failed') { - // we need to delay the visit of the snippets in case they access a ConstTag that is declared - // after the snippets so that the visitor for the const tag can be updated - snippets_visits.push(() => { - /** @type {Statement[]} */ - const init = []; - context.visit(child, { ...context.state, init }); - props.properties.push(b.prop('init', child.expression, child.expression)); - external_statements.push(...init); - }); - } else if (child.type === 'ConstTag') { + if (child.type === 'ConstTag') { + continue; + } + + if (child.type === 'SnippetBlock') { /** @type {Statement[]} */ - const init = []; - context.visit(child, { ...context.state, init }); - - if (dev) { - // In dev we must separate the declarations from the code - // that eagerly evaluate the expression... - for (const statement of init) { - if (statement.type === 'VariableDeclaration') { - external_statements.push(statement); - } else { - internal_statements.push(statement); - } - } - } else { - external_statements.push(...init); + const statements = []; + + context.visit(child, { ...context.state, init: statements }); + + const snippet = /** @type {VariableDeclaration} */ (statements[0]); + + const snippet_fn = dev + ? // @ts-expect-error we know this shape is correct + snippet.declarations[0].init.arguments[1] + : snippet.declarations[0].init; + + snippet_fn.body.body.unshift( + ...const_tags.filter((node) => node.type === 'VariableDeclaration') + ); + + hoisted.push(snippet); + + if (child.expression.name === 'failed') { + props.properties.push(b.prop('init', child.expression, child.expression)); } - } else { - nodes.push(child); + + continue; } - } - snippets_visits.forEach((visit) => visit()); + nodes.push(child); + } const block = /** @type {BlockStatement} */ (context.visit({ ...node.fragment, nodes })); - if (dev && internal_statements.length) { - block.body.unshift(...internal_statements); - } + block.body.unshift(...const_tags); const boundary = b.stmt( b.call('$.boundary', context.state.node, props, b.arrow([b.id('$$anchor')], block)) ); context.state.template.push_comment(); - context.state.init.push( - external_statements.length > 0 ? b.block([...external_statements, boundary]) : boundary - ); + context.state.init.push(hoisted.length > 0 ? b.block([...hoisted, boundary]) : boundary); } diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index c462b207cecf..2e5c31b7a1ab 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -81,7 +81,7 @@ export function adjust_error(error, effect) { if (adjusted_errors.has(error)) return; adjusted_errors.add(error); - const component_stack = [effect.fn?.name ?? '']; + const component_stack = [effect.fn?.name || '']; const indent = is_firefox ? ' ' : '\t'; var context = effect.ctx; From 063eab8ce29b2a1757a116c3663efea642edddac Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:18:43 -0400 Subject: [PATCH 17/29] unused --- .../internal/client/dom/blocks/boundary.js | 5 +---- .../src/internal/client/error-handling.js | 19 +------------------ 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 2387b22f5198..20839a778368 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -2,7 +2,7 @@ import { BOUNDARY_EFFECT, EFFECT_TRANSPARENT } from '#client/constants'; import { component_context, set_component_context } from '../../context.js'; -import { invoke_error_boundary, reset_is_throwing_error } from '../../error-handling.js'; +import { invoke_error_boundary } from '../../error-handling.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; import { active_effect, @@ -80,7 +80,6 @@ export function boundary(node, props, boundary_fn) { with_boundary(boundary, () => { is_creating_fallback = false; boundary_effect = branch(() => boundary_fn(anchor)); - reset_is_throwing_error(); }); }; @@ -119,7 +118,6 @@ export function boundary(node, props, boundary_fn) { invoke_error_boundary(error, /** @type {Effect} */ (boundary.parent)); } - reset_is_throwing_error(); is_creating_fallback = false; }); }); @@ -131,7 +129,6 @@ export function boundary(node, props, boundary_fn) { } boundary_effect = branch(() => boundary_fn(anchor)); - reset_is_throwing_error(); }, EFFECT_TRANSPARENT | BOUNDARY_EFFECT); if (hydrating) { diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 2e5c31b7a1ab..c27ba1a3607f 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -2,19 +2,13 @@ import { DEV } from 'esm-env'; import { FILENAME } from '../../constants.js'; import { is_firefox } from './dom/operations.js'; -import { BOUNDARY_EFFECT, DESTROYED } from './constants.js'; +import { BOUNDARY_EFFECT } from './constants.js'; import { define_property } from '../shared/utils.js'; // Used for DEV time error handling /** @param {WeakSet} value */ const adjusted_errors = new WeakSet(); -let is_throwing_error = false; - -export function reset_is_throwing_error() { - is_throwing_error = false; -} - /** @type {null | unknown} */ let current_error = null; @@ -58,20 +52,9 @@ export function invoke_error_boundary(error, effect) { current = current.parent; } - is_throwing_error = false; throw error; } -/** - * @param {Effect} effect - */ -function should_rethrow_error(effect) { - return ( - (effect.f & DESTROYED) === 0 && - (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) - ); -} - /** * Add useful information to the error message/stack * @param {Error} error From 47372dfb16848ef1932ae5efd607caf744ee4658 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:19:51 -0400 Subject: [PATCH 18/29] unused --- packages/svelte/src/internal/client/error-handling.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index c27ba1a3607f..9613cfa798a0 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -9,19 +9,11 @@ import { define_property } from '../shared/utils.js'; /** @param {WeakSet} value */ const adjusted_errors = new WeakSet(); -/** @type {null | unknown} */ -let current_error = null; - /** * @param {unknown} error * @param {Effect} effect */ export function handle_error(error, effect) { - if (error === current_error) { - // TODO is this necessary? - return; - } - if (DEV && error instanceof Error) { adjust_error(error, effect); } From 786b50a48d42269499d9a373ab619671ff49218c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:24:54 -0400 Subject: [PATCH 19/29] tidy up --- .../src/internal/client/error-handling.js | 19 +++++++++---- .../svelte/src/internal/client/runtime.js | 27 +++++-------------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 9613cfa798a0..66e93bea102d 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -2,8 +2,9 @@ import { DEV } from 'esm-env'; import { FILENAME } from '../../constants.js'; import { is_firefox } from './dom/operations.js'; -import { BOUNDARY_EFFECT } from './constants.js'; +import { BOUNDARY_EFFECT, EFFECT_RAN } from './constants.js'; import { define_property } from '../shared/utils.js'; +import { active_effect } from './runtime.js'; // Used for DEV time error handling /** @param {WeakSet} value */ @@ -11,14 +12,22 @@ const adjusted_errors = new WeakSet(); /** * @param {unknown} error - * @param {Effect} effect */ -export function handle_error(error, effect) { +export function handle_error(error) { + var effect = /** @type {Effect} */ (active_effect); + if (DEV && error instanceof Error) { adjust_error(error, effect); } - invoke_error_boundary(error, effect); + if ((effect.f & EFFECT_RAN) !== 0) { + invoke_error_boundary(error, effect); + } else if ((effect.f & BOUNDARY_EFFECT) !== 0) { + // invoke directly + effect.fn(error); + } else { + throw error; + } } /** @@ -52,7 +61,7 @@ export function invoke_error_boundary(error, effect) { * @param {Error} error * @param {Effect} effect */ -export function adjust_error(error, effect) { +function adjust_error(error, effect) { if (adjusted_errors.has(error)) return; adjusted_errors.add(error); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 0199f2206923..a60a25ad06e5 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1,4 +1,4 @@ -/** @import { ComponentContext, Derived, Effect, Reaction, Signal, Source, Value } from '#client' */ +/** @import { Derived, Effect, Reaction, Signal, Source, Value } from '#client' */ import { DEV } from 'esm-env'; import { define_property, get_descriptors, get_prototype_of, index_of } from '../shared/utils.js'; import { @@ -22,9 +22,7 @@ import { ROOT_EFFECT, LEGACY_DERIVED_PROP, DISCONNECTED, - BOUNDARY_EFFECT, - EFFECT_IS_UPDATING, - EFFECT_RAN + EFFECT_IS_UPDATING } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { internal_set, old_values } from './reactivity/sources.js'; @@ -40,7 +38,7 @@ import { set_component_context, set_dev_current_component_function } from './context.js'; -import { adjust_error, handle_error, invoke_error_boundary } from './error-handling.js'; +import { handle_error, invoke_error_boundary } from './error-handling.js'; let is_flushing = false; @@ -345,20 +343,7 @@ export function update_reaction(reaction) { return result; } catch (error) { - var effect = /** @type {Effect} */ (active_effect); - - if (DEV && error instanceof Error) { - adjust_error(error, effect); - } - - if ((effect.f & EFFECT_RAN) !== 0) { - invoke_error_boundary(error, effect); - } else if ((effect.f & BOUNDARY_EFFECT) !== 0) { - // invoke directly - effect.fn(error); - } else { - throw error; - } + handle_error(error); } finally { new_deps = previous_deps; skipped_deps = previous_skipped_deps; @@ -532,14 +517,14 @@ function infinite_loop_guard() { if (last_scheduled_effect !== null) { if (DEV) { try { - handle_error(error, last_scheduled_effect); + invoke_error_boundary(error, last_scheduled_effect); } catch (e) { // Only log the effect stack if the error is re-thrown log_effect_stack(); throw e; } } else { - handle_error(error, last_scheduled_effect); + invoke_error_boundary(error, last_scheduled_effect); } } else { if (DEV) { From 0e091157a9a7b5298972a4bd9849948a74b42139 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:32:00 -0400 Subject: [PATCH 20/29] unused --- packages/svelte/src/internal/client/error-handling.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 66e93bea102d..9f0838e8cb5e 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -44,10 +44,7 @@ export function invoke_error_boundary(error, effect) { // @ts-expect-error current.fn(error); return; - } catch { - // Remove boundary flag from effect (TODO is this still useful?) - current.f ^= BOUNDARY_EFFECT; - } + } catch {} } current = current.parent; From 0bbceee45f6057d65ae1875ece8ec9dcb1d8ef06 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:43:33 -0400 Subject: [PATCH 21/29] unused --- packages/svelte/src/internal/client/error-handling.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 9f0838e8cb5e..367606d7582d 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -76,11 +76,6 @@ function adjust_error(error, effect) { value: error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` }); - // TODO what is this for? can we get rid of it? - define_property(error, 'component_stack', { - value: component_stack - }); - // Filter out internal files from callstack if (error.stack) { define_property(error, 'stack', { From e243091d461acc0a7c08b05165a73faff8effb8f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 11:47:29 -0400 Subject: [PATCH 22/29] tweak --- packages/svelte/src/internal/client/error-handling.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 367606d7582d..a13c205166ac 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -62,22 +62,21 @@ function adjust_error(error, effect) { if (adjusted_errors.has(error)) return; adjusted_errors.add(error); - const component_stack = [effect.fn?.name || '']; - const indent = is_firefox ? ' ' : '\t'; - + var indent = is_firefox ? ' ' : '\t'; + var component_stack = `\n${indent}in ${effect.fn?.name || ''}`; var context = effect.ctx; while (context !== null) { - component_stack.push(context.function?.[FILENAME].split('/').pop()); + component_stack += `\n${indent}in ${context.function?.[FILENAME].split('/').pop()}`; context = context.p; } define_property(error, 'message', { - value: error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` + value: error.message + `\n${component_stack}\n` }); - // Filter out internal files from callstack if (error.stack) { + // Filter out internal modules define_property(error, 'stack', { value: error.stack .split('\n') From 91458a012d1334407eb9c75a6b0a86c9db202fff Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:17:23 -0400 Subject: [PATCH 23/29] WIP --- .../svelte/src/internal/client/error-handling.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index a13c205166ac..be99c5efda4c 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -20,13 +20,18 @@ export function handle_error(error) { adjust_error(error, effect); } - if ((effect.f & EFFECT_RAN) !== 0) { - invoke_error_boundary(error, effect); - } else if ((effect.f & BOUNDARY_EFFECT) !== 0) { - // invoke directly + if ((effect.f & EFFECT_RAN) === 0) { + // if the error occurred while creating this subtree, we let it + // bubble up until it hits a boundary that can handle it + if ((effect.f & BOUNDARY_EFFECT) === 0) { + throw error; + } + + // @ts-expect-error effect.fn(error); } else { - throw error; + // otherwise we bubble up the effect tree ourselves + invoke_error_boundary(error, effect); } } From 0a433d92c6e95f267c8b3bb5be2e8a47b9ac1618 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:20:33 -0400 Subject: [PATCH 24/29] tweak --- .../svelte/src/internal/client/error-handling.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index be99c5efda4c..8baa1eced55c 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -37,22 +37,19 @@ export function handle_error(error) { /** * @param {unknown} error - * @param {Effect} effect + * @param {Effect | null} effect */ export function invoke_error_boundary(error, effect) { - /** @type {Effect | null} */ - var current = effect; - - while (current !== null) { - if ((current.f & BOUNDARY_EFFECT) !== 0) { + while (effect !== null) { + if ((effect.f & BOUNDARY_EFFECT) !== 0) { try { // @ts-expect-error - current.fn(error); + effect.fn(error); return; } catch {} } - current = current.parent; + effect = effect.parent; } throw error; From a45aab670e0fd61920e82628d7ca2971a68e9546 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:23:07 -0400 Subject: [PATCH 25/29] asked and answered --- packages/svelte/src/internal/client/dom/blocks/boundary.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 20839a778368..d7a53da1d131 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -74,7 +74,6 @@ export function boundary(node, props, boundary_fn) { } var reset = () => { - // TODO does this make sense? pause_effect(boundary_effect); with_boundary(boundary, () => { From 9f0c1f7950dec891d213f199506715df46908cab Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:25:26 -0400 Subject: [PATCH 26/29] tweak --- packages/svelte/src/internal/client/error-handling.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index 8baa1eced55c..aeec1d8b473d 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -6,10 +6,6 @@ import { BOUNDARY_EFFECT, EFFECT_RAN } from './constants.js'; import { define_property } from '../shared/utils.js'; import { active_effect } from './runtime.js'; -// Used for DEV time error handling -/** @param {WeakSet} value */ -const adjusted_errors = new WeakSet(); - /** * @param {unknown} error */ @@ -55,8 +51,11 @@ export function invoke_error_boundary(error, effect) { throw error; } +/** @type {WeakSet} */ +const adjusted_errors = new WeakSet(); + /** - * Add useful information to the error message/stack + * Add useful information to the error message/stack in development * @param {Error} error * @param {Effect} effect */ From 610c252964769c2cebeeaa98d4bc0bbba18db202 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:26:18 -0400 Subject: [PATCH 27/29] unused --- packages/svelte/src/internal/client/runtime.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a60a25ad06e5..ceb145779884 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -358,18 +358,6 @@ 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 From eedaa258b6400ad2247cc1faf41de180c4f13df2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:28:06 -0400 Subject: [PATCH 28/29] changeset --- .changeset/early-tips-prove.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/early-tips-prove.md diff --git a/.changeset/early-tips-prove.md b/.changeset/early-tips-prove.md new file mode 100644 index 000000000000..2df03126d890 --- /dev/null +++ b/.changeset/early-tips-prove.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: invoke parent boundary of deriveds that throw From da2c8a9611657590e38d5ca6d6dc63067da55ba3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 6 Jun 2025 12:52:44 -0400 Subject: [PATCH 29/29] lint --- packages/svelte/src/internal/client/runtime.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index ceb145779884..5920a05fc50d 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -248,11 +248,7 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true) } } -/** - * @template V - * @param {Reaction} reaction - * @returns {V} - */ +/** @param {Reaction} reaction */ export function update_reaction(reaction) { var previous_deps = new_deps; var previous_skipped_deps = skipped_deps;