Skip to content

Commit 9e1e713

Browse files
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 <[email protected]>
1 parent b8b662a commit 9e1e713

File tree

17 files changed

+315
-27
lines changed

17 files changed

+315
-27
lines changed

.changeset/new-dogs-obey.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: handle error in correct boundary after reset

.changeset/polite-toys-report.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: make `<svelte:boundary>` reset function a noop after the first call

documentation/docs/98-reference/.generated/client-errors.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,3 +238,23 @@ let odd = $derived(!even);
238238
```
239239

240240
If side-effects are unavoidable, use [`$effect`]($effect) instead.
241+
242+
### svelte_boundary_reset_onerror
243+
244+
```
245+
A `<svelte:boundary>` `reset` function cannot be called while an error is still being handled
246+
```
247+
248+
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.
249+
250+
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):
251+
252+
```svelte
253+
<svelte:boundary onerror={async (error, reset) => {
254+
fixTheError();
255+
+++await tick();+++
256+
reset();
257+
}}>
258+
259+
</svelte:boundary>
260+
```

documentation/docs/98-reference/.generated/client-warnings.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,32 @@ Reactive `$state(...)` proxies and the values they proxy have different identiti
312312
313313
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.
314314
315+
### svelte_boundary_reset_noop
316+
317+
```
318+
A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called
319+
```
320+
321+
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.
322+
323+
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.
324+
325+
```svelte
326+
<script>
327+
let reset;
328+
</script>
329+
330+
<button onclick={reset}>reset</button>
331+
332+
<svelte:boundary onerror={(e, r) => (reset = r)}>
333+
<!-- contents -->
334+
335+
{#snippet failed(e)}
336+
<p>oops! {e.message}</p>
337+
{/snippet}
338+
</svelte:boundary>
339+
```
340+
315341
### transition_slide_display
316342
317343
```

packages/svelte/messages/client-errors/errors.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,21 @@ let odd = $derived(!even);
184184
```
185185

186186
If side-effects are unavoidable, use [`$effect`]($effect) instead.
187+
188+
## svelte_boundary_reset_onerror
189+
190+
> A `<svelte:boundary>` `reset` function cannot be called while an error is still being handled
191+
192+
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.
193+
194+
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):
195+
196+
```svelte
197+
<svelte:boundary onerror={async (error, reset) => {
198+
fixTheError();
199+
+++await tick();+++
200+
reset();
201+
}}>
202+
203+
</svelte:boundary>
204+
```

packages/svelte/messages/client-warnings/warnings.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,30 @@ To silence the warning, ensure that `value`:
272272
273273
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.
274274
275+
## svelte_boundary_reset_noop
276+
277+
> A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called
278+
279+
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.
280+
281+
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.
282+
283+
```svelte
284+
<script>
285+
let reset;
286+
</script>
287+
288+
<button onclick={reset}>reset</button>
289+
290+
<svelte:boundary onerror={(e, r) => (reset = r)}>
291+
<!-- contents -->
292+
293+
{#snippet failed(e)}
294+
<p>oops! {e.message}</p>
295+
{/snippet}
296+
</svelte:boundary>
297+
```
298+
275299
## transition_slide_display
276300
277301
> The `slide` transition does not work correctly for elements with `display: %value%`

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

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
/** @import { Effect, Source, TemplateNode, } from '#client' */
2-
import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants';
2+
import {
3+
BOUNDARY_EFFECT,
4+
EFFECT_PRESERVED,
5+
EFFECT_RAN,
6+
EFFECT_TRANSPARENT
7+
} from '#client/constants';
38
import { component_context, set_component_context } from '../../context.js';
4-
import { invoke_error_boundary } from '../../error-handling.js';
9+
import { handle_error, invoke_error_boundary } from '../../error-handling.js';
510
import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js';
611
import {
712
active_effect,
@@ -21,6 +26,7 @@ import {
2126
import { get_next_sibling } from '../operations.js';
2227
import { queue_micro_task } from '../task.js';
2328
import * as e from '../../errors.js';
29+
import * as w from '../../warnings.js';
2430
import { DEV } from 'esm-env';
2531
import { Batch, effect_pending_updates } from '../../reactivity/batch.js';
2632
import { internal_set, source } from '../../reactivity/sources.js';
@@ -196,6 +202,9 @@ export class Boundary {
196202

197203
try {
198204
return fn();
205+
} catch (e) {
206+
handle_error(e);
207+
return null;
199208
} finally {
200209
set_active_effect(previous_effect);
201210
set_active_reaction(previous_reaction);
@@ -257,7 +266,42 @@ export class Boundary {
257266
var onerror = this.#props.onerror;
258267
let failed = this.#props.failed;
259268

269+
if (this.#main_effect) {
270+
destroy_effect(this.#main_effect);
271+
this.#main_effect = null;
272+
}
273+
274+
if (this.#pending_effect) {
275+
destroy_effect(this.#pending_effect);
276+
this.#pending_effect = null;
277+
}
278+
279+
if (this.#failed_effect) {
280+
destroy_effect(this.#failed_effect);
281+
this.#failed_effect = null;
282+
}
283+
284+
if (hydrating) {
285+
set_hydrate_node(this.#hydrate_open);
286+
next();
287+
set_hydrate_node(remove_nodes());
288+
}
289+
290+
var did_reset = false;
291+
var calling_on_error = false;
292+
260293
const reset = () => {
294+
if (did_reset) {
295+
w.svelte_boundary_reset_noop();
296+
return;
297+
}
298+
299+
did_reset = true;
300+
301+
if (calling_on_error) {
302+
e.svelte_boundary_reset_onerror();
303+
}
304+
261305
this.#pending_count = 0;
262306

263307
if (this.#failed_effect !== null) {
@@ -290,32 +334,15 @@ export class Boundary {
290334

291335
try {
292336
set_active_reaction(null);
337+
calling_on_error = true;
293338
onerror?.(error, reset);
339+
calling_on_error = false;
340+
} catch (error) {
341+
invoke_error_boundary(error, this.#effect && this.#effect.parent);
294342
} finally {
295343
set_active_reaction(previous_reaction);
296344
}
297345

298-
if (this.#main_effect) {
299-
destroy_effect(this.#main_effect);
300-
this.#main_effect = null;
301-
}
302-
303-
if (this.#pending_effect) {
304-
destroy_effect(this.#pending_effect);
305-
this.#pending_effect = null;
306-
}
307-
308-
if (this.#failed_effect) {
309-
destroy_effect(this.#failed_effect);
310-
this.#failed_effect = null;
311-
}
312-
313-
if (hydrating) {
314-
set_hydrate_node(this.#hydrate_open);
315-
next();
316-
set_hydrate_node(remove_nodes());
317-
}
318-
319346
if (failed) {
320347
queue_micro_task(() => {
321348
this.#failed_effect = this.#run(() => {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ export function invoke_error_boundary(error, effect) {
5353
try {
5454
/** @type {Boundary} */ (effect.b).error(error);
5555
return;
56-
} catch {}
56+
} catch (e) {
57+
error = e;
58+
}
5759
}
5860

5961
effect = effect.parent;

packages/svelte/src/internal/client/errors.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,4 +423,20 @@ export function state_unsafe_mutation() {
423423
} else {
424424
throw new Error(`https://svelte.dev/e/state_unsafe_mutation`);
425425
}
426+
}
427+
428+
/**
429+
* A `<svelte:boundary>` `reset` function cannot be called while an error is still being handled
430+
* @returns {never}
431+
*/
432+
export function svelte_boundary_reset_onerror() {
433+
if (DEV) {
434+
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`);
435+
436+
error.name = 'Svelte error';
437+
438+
throw error;
439+
} else {
440+
throw new Error(`https://svelte.dev/e/svelte_boundary_reset_onerror`);
441+
}
426442
}

packages/svelte/src/internal/client/warnings.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,17 @@ export function state_proxy_equality_mismatch(operator) {
224224
}
225225
}
226226

227+
/**
228+
* A `<svelte:boundary>` `reset` function only resets the boundary the first time it is called
229+
*/
230+
export function svelte_boundary_reset_noop() {
231+
if (DEV) {
232+
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);
233+
} else {
234+
console.warn(`https://svelte.dev/e/svelte_boundary_reset_noop`);
235+
}
236+
}
237+
227238
/**
228239
* The `slide` transition does not work correctly for elements with `display: %value%`
229240
* @param {string} value

0 commit comments

Comments
 (0)