diff --git a/.changeset/clean-cats-destroy.md b/.changeset/clean-cats-destroy.md new file mode 100644 index 0000000000..6255318dd4 --- /dev/null +++ b/.changeset/clean-cats-destroy.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +Run `onDestroy` callbacks when server rendering fails diff --git a/packages/svelte/src/internal/server/renderer.js b/packages/svelte/src/internal/server/renderer.js index 35aac64721..622cc70e98 100644 --- a/packages/svelte/src/internal/server/renderer.js +++ b/packages/svelte/src/internal/server/renderer.js @@ -197,9 +197,11 @@ export class Renderer { * Create a child renderer. The child renderer inherits the state from the parent, * but has its own content. * @param {(renderer: Renderer) => MaybePromise} fn + * @param {boolean} [is_component_body] */ - child(fn) { + child(fn, is_component_body = false) { const child = new Renderer(this.global, this); + child.#is_component_body = is_component_body; this.#out.push(child); const parent = ssr_context; @@ -316,9 +318,11 @@ export class Renderer { */ component(fn, component_fn) { push(component_fn); - const child = this.child(fn); - child.#is_component_body = true; - pop(); + try { + this.child(fn, true); + } finally { + pop(); + } } /** @@ -634,12 +638,18 @@ export class Renderer { */ static #render(component, options) { var previous_context = ssr_context; + /** @type {Renderer | undefined} */ + let renderer; + try { - const renderer = Renderer.#open_render('sync', component, options); + renderer = Renderer.#open_render('sync', component, options); const content = renderer.#collect_content(); return Renderer.#close_render(content, renderer); } finally { + if (renderer) { + renderer.#destroy(); + } abort(); set_ssr_context(previous_context); } @@ -655,9 +665,11 @@ export class Renderer { */ static async #render_async(component, options) { const previous_context = ssr_context; + /** @type {Renderer | undefined} */ + let renderer; try { - const renderer = Renderer.#open_render('async', component, options); + renderer = Renderer.#open_render('async', component, options); const content = await renderer.#collect_content_async(); const hydratables = await renderer.#collect_hydratables(); if (hydratables !== null) { @@ -665,6 +677,9 @@ export class Renderer { } return Renderer.#close_render(content, renderer); } finally { + if (renderer) { + renderer.#destroy(); + } set_ssr_context(previous_context); abort(); } @@ -785,8 +800,13 @@ export class Renderer { set_ssr_context(context); renderer.push(BLOCK_OPEN); - // @ts-expect-error - component(renderer, options.props ?? {}); + try { + // @ts-expect-error + component(renderer, options.props ?? {}); + } catch (error) { + renderer.#destroy(); + throw error; + } renderer.push(BLOCK_CLOSE); return renderer; @@ -801,10 +821,6 @@ export class Renderer { * @returns {AccumulatedContent & { hashes: { script: Sha256Source[] } }} */ static #close_render(content, renderer) { - for (const cleanup of renderer.#collect_on_destroy()) { - cleanup(); - } - let head = content.head + renderer.global.get_title(); let body = content.body; @@ -821,6 +837,12 @@ export class Renderer { }; } + #destroy() { + for (const cleanup of this.#collect_on_destroy()) { + cleanup(); + } + } + /** * @param {HydratableContext} ctx */ diff --git a/packages/svelte/src/internal/server/renderer.test.ts b/packages/svelte/src/internal/server/renderer.test.ts index 8e98c41796..22c602dfdd 100644 --- a/packages/svelte/src/internal/server/renderer.test.ts +++ b/packages/svelte/src/internal/server/renderer.test.ts @@ -466,4 +466,31 @@ describe('async', () => { await Renderer.render(component as unknown as Component); expect(destroyed).toEqual(['c', 'e', 'a', 'b', 'b*', 'd']); }); + + test('on_destroy runs when synchronous rendering fails', () => { + const destroyed: string[] = []; + const component = (renderer: Renderer) => { + renderer.component((renderer) => { + renderer.on_destroy(() => destroyed.push('destroyed')); + throw new Error('boom'); + }); + }; + + expect(() => Renderer.render(component as unknown as Component).body).toThrow('boom'); + expect(destroyed).toEqual(['destroyed']); + }); + + test('on_destroy runs when asynchronous rendering fails', async () => { + const destroyed: string[] = []; + const component = (renderer: Renderer) => { + renderer.component(async (renderer) => { + renderer.on_destroy(() => destroyed.push('destroyed')); + await Promise.resolve(); + throw new Error('boom'); + }); + }; + + await expect(Renderer.render(component as unknown as Component)).rejects.toThrow('boom'); + expect(destroyed).toEqual(['destroyed']); + }); });