diff --git a/.changeset/new-dogs-obey.md b/.changeset/new-dogs-obey.md new file mode 100644 index 000000000000..aa9a3d73b958 --- /dev/null +++ b/.changeset/new-dogs-obey.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: handle error in correct boundary after reset diff --git a/.changeset/polite-toys-report.md b/.changeset/polite-toys-report.md new file mode 100644 index 000000000000..bf5386e06916 --- /dev/null +++ b/.changeset/polite-toys-report.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: make `` reset function a noop after the first call diff --git a/documentation/docs/98-reference/.generated/client-errors.md b/documentation/docs/98-reference/.generated/client-errors.md index 3b17ef9f9b4e..2868fbeb6500 100644 --- a/documentation/docs/98-reference/.generated/client-errors.md +++ b/documentation/docs/98-reference/.generated/client-errors.md @@ -238,3 +238,23 @@ let odd = $derived(!even); ``` If side-effects are unavoidable, use [`$effect`]($effect) instead. + +### svelte_boundary_reset_onerror + +``` +A `` `reset` function cannot be called while an error is still being handled +``` + +If a [``](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 + { + fixTheError(); + +++await tick();+++ + reset(); +}}> + + +``` diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index 7548428e9784..6f1d677fe951 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -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 `` `reset` function only resets the boundary the first time it is called +``` + +When an error occurs while rendering the contents of a [``](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 `` is rendered will _not_ cause the contents to be rendered again. + +```svelte + + + + + (reset = r)}> + + + {#snippet failed(e)} +

oops! {e.message}

+ {/snippet} + +``` + ### transition_slide_display ``` diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index d6af8598812b..d711c85c5e16 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -184,3 +184,21 @@ let odd = $derived(!even); ``` If side-effects are unavoidable, use [`$effect`]($effect) instead. + +## svelte_boundary_reset_onerror + +> A `` `reset` function cannot be called while an error is still being handled + +If a [``](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 + { + fixTheError(); + +++await tick();+++ + reset(); +}}> + + +``` diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 13d9bfcd3bcd..123c6833e688 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -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 `` `reset` function only resets the boundary the first time it is called + +When an error occurs while rendering the contents of a [``](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 `` is rendered will _not_ cause the contents to be rendered again. + +```svelte + + + + + (reset = r)}> + + + {#snippet failed(e)} +

oops! {e.message}

+ {/snippet} + +``` + ## transition_slide_display > The `slide` transition does not work correctly for elements with `display: %value%` diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 5e678ab113ce..4ea137bfa8b8 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -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(() => { diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index a512839181d0..6c83a453d540 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -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; diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index edd66a7400d7..937971da5e0b 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -423,4 +423,20 @@ export function state_unsafe_mutation() { } else { throw new Error(`https://svelte.dev/e/state_unsafe_mutation`); } +} + +/** + * A `` `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 \`\` \`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`); + } } \ No newline at end of file diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index dfd50a8722c9..dfa2a3752e52 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -224,6 +224,17 @@ export function state_proxy_equality_mismatch(operator) { } } +/** + * A `` `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 \`\` \`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 diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index 8d7b3544bc6e..05c1a982ec6f 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -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 + } } } } diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-onerror/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-onerror/_config.js new file mode 100644 index 000000000000..092d7ad37d57 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-onerror/_config.js @@ -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, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-onerror/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-onerror/main.svelte new file mode 100644 index 000000000000..f91048a9e778 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-onerror/main.svelte @@ -0,0 +1,17 @@ + + + reset()}> + {must_throw ? throw_error() : 'normal content'} + + {#snippet failed()} +
err
+ {/snippet} + + + diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-premature/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-premature/_config.js new file mode 100644 index 000000000000..687961e721d0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-premature/_config.js @@ -0,0 +1,28 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ` + normal content + + `, + + async test({ assert, target, warnings }) { + const [btn] = target.querySelectorAll('button'); + + flushSync(() => btn.click()); + assert.htmlEqual(target.innerHTML, `
err
`); + assert.deepEqual(warnings, []); + + flushSync(() => btn.click()); + assert.htmlEqual(target.innerHTML, `normal content `); + assert.deepEqual(warnings, []); + + flushSync(() => btn.click()); + assert.htmlEqual(target.innerHTML, `
err
`); + + assert.deepEqual(warnings, [ + 'A `` `reset` function only resets the boundary the first time it is called' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-premature/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-premature/main.svelte new file mode 100644 index 000000000000..c1462eaf09c2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-premature/main.svelte @@ -0,0 +1,26 @@ + + + + (reset = fn)}> + {must_throw ? throw_error() : 'normal content'} + + {#snippet failed()} +
err
+ {/snippet} + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-with-error/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-with-error/_config.js new file mode 100644 index 000000000000..844e6981bd44 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-with-error/_config.js @@ -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, + `

yikes!

` + ); + + const [, reset] = target.querySelectorAll('button'); + flushSync(() => reset.click()); + assert.htmlEqual( + target.innerHTML, + `

yikes!

` + ); + + flushSync(() => toggle.click()); + + const [, reset2] = target.querySelectorAll('button'); + flushSync(() => reset2.click()); + assert.htmlEqual(target.innerHTML, `

hello!

`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-with-error/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-with-error/main.svelte new file mode 100644 index 000000000000..91479a631ade --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-reset-with-error/main.svelte @@ -0,0 +1,20 @@ + + + + + +

{must_throw ? throw_error() : 'hello!'}

+ + {#snippet failed(error, reset)} +

{error.message}

+ + {/snippet} + + +