pull/16197/head
Rich Harris 4 months ago
commit 84e07903a2

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

@ -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,7 +128,11 @@ export class Boundary {
}
});
} else {
try {
this.#main_effect = branch(() => children(this.#anchor));
} catch (error) {
this.error(error);
}
if (this.#pending_count > 0) {
this.#show_pending_snippet();
@ -137,8 +140,6 @@ export class Boundary {
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();
}
});
if (this.#pending_count > 0) {
@ -260,7 +256,14 @@ export class Boundary {
throw error;
}
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;
}
});

@ -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')
});
}
}

@ -20,7 +20,6 @@ import {
update_reaction,
increment_write_version,
set_active_effect,
handle_error,
push_reaction_value,
is_destroying_effect
} from '../runtime.js';
@ -35,6 +34,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;
@ -157,8 +157,16 @@ export function async_derived(fn, location) {
if (ran) batch.restore();
if (error) {
// TODO instead of handling error here, mark the signal as bad and let it happen when it is read
if (error !== STALE_REACTION) {
handle_error(error, parent, null, parent.ctx);
var previous_effect = active_effect;
try {
set_active_effect(parent);
handle_error(error);
} finally {
set_active_effect(previous_effect);
}
}
} else {
internal_set(signal, value);

@ -361,13 +361,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);
}
@ -455,7 +448,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;
}

@ -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 {
deferred,
@ -60,6 +60,7 @@ import * as w from './warnings.js';
import { is_firefox } from './dom/operations.js';
import { current_batch, Batch, batch_deriveds } from './reactivity/batch.js';
import { log_effect_tree, root } from './dev/debug.js';
import { handle_error, invoke_error_boundary } from './error-handling.js';
// Used for DEV time error handling
/** @param {WeakSet<Error>} value */
@ -256,130 +257,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 {
/** @type {Boundary} */ (current.b).error(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
* @param {ComponentContext | null} component_context
*/
export function handle_error(error, effect, previous_effect, component_context) {
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
@ -407,11 +284,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;
@ -507,6 +380,8 @@ export function update_reaction(reaction) {
}
return result;
} catch (error) {
handle_error(error);
} finally {
reaction.f ^= REACTION_IS_UPDATING;
new_deps = previous_deps;
@ -594,7 +469,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;
@ -637,8 +511,6 @@ export function update_effect(effect) {
if (DEV) {
dev_effect_stack.push(effect);
}
} catch (error) {
handle_error(error, effect, previous_effect, previous_component_context || effect.ctx);
} finally {
is_updating_effect = was_updating_effect;
active_effect = previous_effect;
@ -673,14 +545,14 @@ function infinite_loop_guard() {
if (last_scheduled_effect !== null) {
if (DEV) {
try {
handle_error(error, last_scheduled_effect, null, null);
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, null, null);
invoke_error_boundary(error, last_scheduled_effect);
}
} else {
if (DEV) {
@ -731,7 +603,6 @@ export function flush_queued_effects(effects) {
var effect = effects[i];
if ((effect.f & (DESTROYED | INERT)) === 0) {
try {
if (check_dirtiness(effect)) {
update_effect(effect);
@ -750,9 +621,6 @@ export function flush_queued_effects(effects) {
}
}
}
} catch (error) {
handle_error(error, effect, null, effect.ctx);
}
}
}
}
@ -805,13 +673,9 @@ export function process_effects(batch, root) {
batch.async_effects.push(effect);
}
} else if ((flags & BLOCK_EFFECT) !== 0) {
try {
if (check_dirtiness(effect)) {
update_effect(effect);
}
} catch (error) {
handle_error(error, effect, null, effect.ctx);
}
} else if (is_branch) {
effect.f ^= CLEAN;
} else if ((flags & RENDER_EFFECT) !== 0) {
@ -820,13 +684,9 @@ export function process_effects(batch, root) {
if (async_mode_flag) {
batch.render_effects.push(effect);
} else {
try {
if (check_dirtiness(effect)) {
update_effect(effect);
}
} catch (error) {
handle_error(error, effect, null, effect.ctx);
}
}
} else if ((flags & EFFECT) !== 0) {
batch.effects.push(effect);

@ -5,9 +5,8 @@ export default test({
test({ assert, target }) {
const btn = target.querySelector('button');
btn?.click();
flushSync();
assert.htmlEqual(target.innerHTML, `<button>change</button><p>Error occured</p>`);
assert.throws(() => {
flushSync(() => btn?.click());
}, /kaboom/);
}
});

@ -1,20 +1,18 @@
<script>
let count = $state(0);
const things = $derived.by(() => {
const d = $derived.by(() => {
if (count === 1) {
throw new Error('123')
throw new Error('kaboom')
}
return [1, 2 ,3]
return count
})
</script>
<button onclick={() => count++}>change</button>
<svelte:boundary>
{#each things as thing}
<p>{thing}</p>
{/each}
{d}
{#snippet failed()}
<p>Error occured</p>

@ -1,7 +1,14 @@
<script>
const { things } = $props();
const { count } = $props();
const d = $derived.by(() => {
if (count === 1) {
throw new Error('kaboom')
}
return count
});
$effect(() => {
things
})
d;
});
</script>

@ -8,6 +8,6 @@ export default test({
btn?.click();
flushSync();
assert.htmlEqual(target.innerHTML, `<button>change</button><p>Error occured</p>`);
assert.htmlEqual(target.innerHTML, `<button>change</button><p>Error occurred</p>`);
}
});

@ -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]
})
</script>
<button onclick={() => count++}>change</button>
<svelte:boundary>
<Child {things} />
<Child {count} />
{#snippet failed()}
<p>Error occured</p>
<p>Error occurred</p>
{/snippet}
</svelte:boundary>

@ -2,14 +2,16 @@ import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // TODO unskip once tagged values are in and we can fix this properly
test({ assert, target }) {
let btn = target.querySelector('button');
assert.throws(() => {
flushSync(() => {
btn?.click();
btn?.click();
assert.throws(() => {
flushSync();
});
}, /test\n\n\tin {expression}\n/);
}
});

@ -1,16 +1,18 @@
<script>
let count = $state(0);
let test = $derived.by(() => {
function maybe_throw() {
if (count > 1) {
throw new Error('test');
}
});
return count;
}
</script>
<svelte:boundary onerror={(e) => { throw(e) }}>
<div>Count: {count}</div>
<button onclick={() => count++}>Increment</button>
{count} / {test}
{count} / {maybe_throw()}
</svelte:boundary>

Loading…
Cancel
Save