WIP

Author: Rich-Harris

.changeset/early-tips-prove.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: invoke parent boundary of deriveds that throw

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,65 +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' || child.expression.name === 'pending')
-		) {
-			// 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 (['failed', 'pending'].includes(child.expression.name)) {
+				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);
 }

packages/svelte/src/internal/client/dom/blocks/boundary.js

@@ -2,14 +2,13 @@
 
 import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants';
 import { component_context, set_component_context } from '../../context.js';
+import { invoke_error_boundary } 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,
@@ -129,16 +128,18 @@ export class Boundary {
 					}
 				});
 			} else {
-				this.#main_effect = branch(() => children(this.#anchor));
+				try {
+					this.#main_effect = branch(() => children(this.#anchor));
+				} catch (error) {
+					this.error(error);
+				}
 
 				if (this.#pending_count > 0) {
 					this.#show_pending_snippet();
 				} else {
 					this.ran = true;
 				}
 			}
-
-			reset_is_throwing_error();
 		}, flags);
 
 		if (hydrating) {
@@ -239,12 +240,7 @@ export class Boundary {
 
 			this.#main_effect = this.#run(() => {
 				this.#is_creating_fallback = false;
-
-				try {
-					return branch(() => this.#children(this.#anchor));
-				} finally {
-					reset_is_throwing_error();
-				}
+				return branch(() => this.#children(this.#anchor));
 			});
 
 			if (this.#pending_count > 0) {
@@ -260,7 +256,14 @@ export class Boundary {
 			throw error;
 		}
 
-		onerror?.(error, reset);
+		var previous_reaction = active_reaction;
+
+		try {
+			set_active_reaction(null);
+			onerror?.(error, reset);
+		} finally {
+			set_active_reaction(previous_reaction);
+		}
 
 		if (this.#main_effect) {
 			destroy_effect(this.#main_effect);
@@ -297,10 +300,9 @@ export class Boundary {
 							);
 						});
 					} catch (error) {
-						handle_error(error, this.#effect, null, this.#effect.ctx);
+						invoke_error_boundary(error, /** @type {Effect} */ (this.#effect.parent));
 						return null;
 					} finally {
-						reset_is_throwing_error();
 						this.#is_creating_fallback = false;
 					}
 				});

packages/svelte/src/internal/client/error-handling.js

@@ -0,0 +1,88 @@
+/** @import { Effect } from '#client' */
+/** @import { Boundary } from './dom/blocks/boundary.js' */
+import { DEV } from 'esm-env';
+import { FILENAME } from '../../constants.js';
+import { is_firefox } from './dom/operations.js';
+import { BOUNDARY_EFFECT, EFFECT_RAN } from './constants.js';
+import { define_property } from '../shared/utils.js';
+import { active_effect } from './runtime.js';
+
+/**
+ * @param {unknown} error
+ */
+export function handle_error(error) {
+	var effect = /** @type {Effect} */ (active_effect);
+
+	if (DEV && error instanceof Error) {
+		// adjust_error(error, effect);
+	}
+
+	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 {
+		// otherwise we bubble up the effect tree ourselves
+		invoke_error_boundary(error, effect);
+	}
+}
+
+/**
+ * @param {unknown} error
+ * @param {Effect | null} effect
+ */
+export function invoke_error_boundary(error, effect) {
+	while (effect !== null) {
+		if ((effect.f & BOUNDARY_EFFECT) !== 0) {
+			try {
+				/** @type {Boundary} */ (effect.b).error(error);
+				return;
+			} catch {}
+		}
+
+		effect = effect.parent;
+	}
+
+	throw error;
+}
+
+/** @type {WeakSet<Error>} */
+const adjusted_errors = new WeakSet();
+
+/**
+ * Add useful information to the error message/stack in development
+ * @param {Error} error
+ * @param {Effect} effect
+ */
+function adjust_error(error, effect) {
+	if (adjusted_errors.has(error)) return;
+	adjusted_errors.add(error);
+
+	var indent = is_firefox ? '  ' : '\t';
+	var component_stack = `\n${indent}in ${effect.fn?.name || '<unknown>'}`;
+	var context = effect.ctx;
+
+	while (context !== null) {
+		component_stack += `\n${indent}in ${context.function?.[FILENAME].split('/').pop()}`;
+		context = context.p;
+	}
+
+	define_property(error, 'message', {
+		value: error.message + `\n${component_stack}\n`
+	});
+
+	if (error.stack) {
+		// Filter out internal modules
+		define_property(error, 'stack', {
+			value: error.stack
+				.split('\n')
+				.filter((line) => !line.includes('svelte/src/internal'))
+				.join('\n')
+		});
+	}
+}

packages/svelte/src/internal/client/reactivity/deriveds.js

@@ -35,6 +35,7 @@ import { get_pending_boundary } from '../dom/blocks/boundary.js';
 import { component_context } from '../context.js';
 import { UNINITIALIZED } from '../../../constants.js';
 import { current_batch } from './batch.js';
+import { handle_error } from '../error-handling.js';
 
 /** @type {Effect | null} */
 export let from_async_derived = null;

packages/svelte/src/internal/client/reactivity/effects.js

@@ -367,13 +367,6 @@ export function template_effect(fn, sync = [], async = [], d = derived) {
  */
 function create_template_effect(fn, deriveds) {
 	var effect = () => fn(...deriveds.map(get));
-
-	if (DEV) {
-		define_property(effect, 'name', {
-			value: '{expression}'
-		});
-	}
-
 	create_effect(RENDER_EFFECT, effect, true);
 }
 
@@ -461,7 +454,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;
 	}