fix: robustify reset calls in error boundaries (#16171)

Fixes #16134

* Add a warning when the misuse of `reset` in an `error:boundary` causes an error to be thrown when flushing the boundary content

* Prevent uncaught errors to make test fails when they are expected and are fired during template effects flush

* reset should just be a noop after the first call

* correctly handle errors inside boundary during reset

* handle errors in the correct boundary

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/16418/head
Ray Thurn Void 2 months ago committed by GitHub
parent b8b662a1ad
commit 9e1e7139fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: handle error in correct boundary after reset

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: make `<svelte:boundary>` reset function a noop after the first call

@ -238,3 +238,23 @@ let odd = $derived(!even);
```
If side-effects are unavoidable, use [`$effect`]($effect) instead.
### svelte_boundary_reset_onerror
```
A `<svelte:boundary>` `reset` function cannot be called while an error is still being handled
```
If a [`<svelte:boundary>`](https://svelte.dev/docs/svelte/svelte-boundary) has an `onerror` function, it must not call the provided `reset` function synchronously since the boundary is still in a broken state. Typically, `reset()` is called later, once the error has been resolved.
If it's possible to resolve the error inside the `onerror` callback, you must at least wait for the boundary to settle before calling `reset()`, for example using [`tick`](https://svelte.dev/docs/svelte/lifecycle-hooks#tick):
```svelte
<svelte:boundary onerror={async (error, reset) => {
fixTheError();
+++await tick();+++
reset();
}}>
</svelte:boundary>
```

@ -312,6 +312,32 @@ Reactive `$state(...)` proxies and the values they proxy have different identiti
To resolve this, ensure you're comparing values where both values were created with `$state(...)`, or neither were. Note that `$state.raw(...)` will _not_ create a state proxy.
### svelte_boundary_reset_noop
```
A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called
```
When an error occurs while rendering the contents of a [`<svelte:boundary>`](https://svelte.dev/docs/svelte/svelte-boundary), the `onerror` handler is called with the error plus a `reset` function that attempts to re-render the contents.
This `reset` function should only be called once. After that, it has no effect — in a case like this, where a reference to `reset` is stored outside the boundary, clicking the button while `<Contents />` is rendered will _not_ cause the contents to be rendered again.
```svelte
<script>
let reset;
</script>
<button onclick={reset}>reset</button>
<svelte:boundary onerror={(e, r) => (reset = r)}>
<!-- contents -->
{#snippet failed(e)}
<p>oops! {e.message}</p>
{/snippet}
</svelte:boundary>
```
### transition_slide_display
```

@ -184,3 +184,21 @@ let odd = $derived(!even);
```
If side-effects are unavoidable, use [`$effect`]($effect) instead.
## svelte_boundary_reset_onerror
> A `<svelte:boundary>` `reset` function cannot be called while an error is still being handled
If a [`<svelte:boundary>`](https://svelte.dev/docs/svelte/svelte-boundary) has an `onerror` function, it must not call the provided `reset` function synchronously since the boundary is still in a broken state. Typically, `reset()` is called later, once the error has been resolved.
If it's possible to resolve the error inside the `onerror` callback, you must at least wait for the boundary to settle before calling `reset()`, for example using [`tick`](https://svelte.dev/docs/svelte/lifecycle-hooks#tick):
```svelte
<svelte:boundary onerror={async (error, reset) => {
fixTheError();
+++await tick();+++
reset();
}}>
</svelte:boundary>
```

@ -272,6 +272,30 @@ To silence the warning, ensure that `value`:
To resolve this, ensure you're comparing values where both values were created with `$state(...)`, or neither were. Note that `$state.raw(...)` will _not_ create a state proxy.
## svelte_boundary_reset_noop
> A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called
When an error occurs while rendering the contents of a [`<svelte:boundary>`](https://svelte.dev/docs/svelte/svelte-boundary), the `onerror` handler is called with the error plus a `reset` function that attempts to re-render the contents.
This `reset` function should only be called once. After that, it has no effect — in a case like this, where a reference to `reset` is stored outside the boundary, clicking the button while `<Contents />` is rendered will _not_ cause the contents to be rendered again.
```svelte
<script>
let reset;
</script>
<button onclick={reset}>reset</button>
<svelte:boundary onerror={(e, r) => (reset = r)}>
<!-- contents -->
{#snippet failed(e)}
<p>oops! {e.message}</p>
{/snippet}
</svelte:boundary>
```
## transition_slide_display
> The `slide` transition does not work correctly for elements with `display: %value%`

@ -1,7 +1,12 @@
/** @import { Effect, Source, TemplateNode, } from '#client' */
import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants';
import {
BOUNDARY_EFFECT,
EFFECT_PRESERVED,
EFFECT_RAN,
EFFECT_TRANSPARENT
} from '#client/constants';
import { component_context, set_component_context } from '../../context.js';
import { invoke_error_boundary } from '../../error-handling.js';
import { handle_error, invoke_error_boundary } from '../../error-handling.js';
import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js';
import {
active_effect,
@ -21,6 +26,7 @@ import {
import { get_next_sibling } from '../operations.js';
import { queue_micro_task } from '../task.js';
import * as e from '../../errors.js';
import * as w from '../../warnings.js';
import { DEV } from 'esm-env';
import { Batch, effect_pending_updates } from '../../reactivity/batch.js';
import { internal_set, source } from '../../reactivity/sources.js';
@ -196,6 +202,9 @@ export class Boundary {
try {
return fn();
} catch (e) {
handle_error(e);
return null;
} finally {
set_active_effect(previous_effect);
set_active_reaction(previous_reaction);
@ -257,7 +266,42 @@ export class Boundary {
var onerror = this.#props.onerror;
let failed = this.#props.failed;
if (this.#main_effect) {
destroy_effect(this.#main_effect);
this.#main_effect = null;
}
if (this.#pending_effect) {
destroy_effect(this.#pending_effect);
this.#pending_effect = null;
}
if (this.#failed_effect) {
destroy_effect(this.#failed_effect);
this.#failed_effect = null;
}
if (hydrating) {
set_hydrate_node(this.#hydrate_open);
next();
set_hydrate_node(remove_nodes());
}
var did_reset = false;
var calling_on_error = false;
const reset = () => {
if (did_reset) {
w.svelte_boundary_reset_noop();
return;
}
did_reset = true;
if (calling_on_error) {
e.svelte_boundary_reset_onerror();
}
this.#pending_count = 0;
if (this.#failed_effect !== null) {
@ -290,32 +334,15 @@ export class Boundary {
try {
set_active_reaction(null);
calling_on_error = true;
onerror?.(error, reset);
calling_on_error = false;
} catch (error) {
invoke_error_boundary(error, this.#effect && this.#effect.parent);
} finally {
set_active_reaction(previous_reaction);
}
if (this.#main_effect) {
destroy_effect(this.#main_effect);
this.#main_effect = null;
}
if (this.#pending_effect) {
destroy_effect(this.#pending_effect);
this.#pending_effect = null;
}
if (this.#failed_effect) {
destroy_effect(this.#failed_effect);
this.#failed_effect = null;
}
if (hydrating) {
set_hydrate_node(this.#hydrate_open);
next();
set_hydrate_node(remove_nodes());
}
if (failed) {
queue_micro_task(() => {
this.#failed_effect = this.#run(() => {

@ -53,7 +53,9 @@ export function invoke_error_boundary(error, effect) {
try {
/** @type {Boundary} */ (effect.b).error(error);
return;
} catch {}
} catch (e) {
error = e;
}
}
effect = effect.parent;

@ -423,4 +423,20 @@ export function state_unsafe_mutation() {
} else {
throw new Error(`https://svelte.dev/e/state_unsafe_mutation`);
}
}
/**
* A `<svelte:boundary>` `reset` function cannot be called while an error is still being handled
* @returns {never}
*/
export function svelte_boundary_reset_onerror() {
if (DEV) {
const error = new Error(`svelte_boundary_reset_onerror\nA \`<svelte:boundary>\` \`reset\` function cannot be called while an error is still being handled\nhttps://svelte.dev/e/svelte_boundary_reset_onerror`);
error.name = 'Svelte error';
throw error;
} else {
throw new Error(`https://svelte.dev/e/svelte_boundary_reset_onerror`);
}
}

@ -224,6 +224,17 @@ export function state_proxy_equality_mismatch(operator) {
}
}
/**
* A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called
*/
export function svelte_boundary_reset_noop() {
if (DEV) {
console.warn(`%c[svelte] svelte_boundary_reset_noop\n%cA \`<svelte:boundary>\` \`reset\` function only resets the boundary the first time it is called\nhttps://svelte.dev/e/svelte_boundary_reset_noop`, bold, normal);
} else {
console.warn(`https://svelte.dev/e/svelte_boundary_reset_noop`);
}
}
/**
* The `slide` transition does not work correctly for elements with `display: %value%`
* @param {string} value

@ -489,10 +489,11 @@ async function run_test_variant(
'Expected component to unmount and leave nothing behind after it was destroyed'
);
// TODO: This seems useless, unhandledRejection is only triggered on the next task
// by which time the test has already finished and the next test resets it to null above
// uncaught errors like during template effects flush
if (unhandled_rejection) {
throw unhandled_rejection; // eslint-disable-line no-unsafe-finally
if (!config.expect_unhandled_rejections) {
throw unhandled_rejection; // eslint-disable-line no-unsafe-finally
}
}
}
}

@ -0,0 +1,15 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
test({ assert, target }) {
const btn = target.querySelector('button');
btn?.click();
assert.throws(flushSync, 'svelte_boundary_reset_onerror');
// boundary content empty; only button remains
assert.htmlEqual(target.innerHTML, `<button>trigger throw</button>`);
}
});

@ -0,0 +1,17 @@
<script>
let must_throw = $state(false);
function throw_error() {
throw new Error("error on template render");
}
</script>
<svelte:boundary onerror={(_, reset) => reset()}>
{must_throw ? throw_error() : 'normal content'}
{#snippet failed()}
<div>err</div>
{/snippet}
</svelte:boundary>
<button onclick={() => must_throw = true}>trigger throw</button>

@ -0,0 +1,28 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
html: `
normal content
<button>toggle</button>
`,
async test({ assert, target, warnings }) {
const [btn] = target.querySelectorAll('button');
flushSync(() => btn.click());
assert.htmlEqual(target.innerHTML, `<div>err</div><button>toggle</button>`);
assert.deepEqual(warnings, []);
flushSync(() => btn.click());
assert.htmlEqual(target.innerHTML, `normal content <button>toggle</button>`);
assert.deepEqual(warnings, []);
flushSync(() => btn.click());
assert.htmlEqual(target.innerHTML, `<div>err</div><button>toggle</button>`);
assert.deepEqual(warnings, [
'A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called'
]);
}
});

@ -0,0 +1,26 @@
<script>
let must_throw = $state(false);
let reset = $state(null);
function throw_error() {
throw new Error("error on template render");
}
</script>
<svelte:boundary onerror={console.error}>
<svelte:boundary onerror={(_, fn) => (reset = fn)}>
{must_throw ? throw_error() : 'normal content'}
{#snippet failed()}
<div>err</div>
{/snippet}
</svelte:boundary>
</svelte:boundary>
<button
onclick={() => {
must_throw = !must_throw;
if (reset) reset();
}}>
toggle
</button>

@ -0,0 +1,27 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
test({ assert, target, warnings }) {
const [toggle] = target.querySelectorAll('button');
flushSync(() => toggle.click());
assert.htmlEqual(
target.innerHTML,
`<button>toggle</button><p>yikes!</p><button>reset</button>`
);
const [, reset] = target.querySelectorAll('button');
flushSync(() => reset.click());
assert.htmlEqual(
target.innerHTML,
`<button>toggle</button><p>yikes!</p><button>reset</button>`
);
flushSync(() => toggle.click());
const [, reset2] = target.querySelectorAll('button');
flushSync(() => reset2.click());
assert.htmlEqual(target.innerHTML, `<button>toggle</button><p>hello!</p>`);
}
});

@ -0,0 +1,20 @@
<script>
let must_throw = $state(false);
function throw_error() {
throw new Error('yikes!');
}
</script>
<button onclick={() => must_throw = !must_throw}>toggle</button>
<svelte:boundary>
<p>{must_throw ? throw_error() : 'hello!'}</p>
{#snippet failed(error, reset)}
<p>{error.message}</p>
<button onclick={reset}>reset</button>
{/snippet}
</svelte:boundary>
Loading…
Cancel
Save