msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

Merge 9bd44762b4 into 44a7813730

Browse Source
pull/18585/merge
Nic Polumeyv 1 week ago committed by GitHub
parent 44a7813730 9bd44762b4
commit e0caefb126
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 209 additions and 28 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/great-hoops-tickle.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: run `onDestroy` callbacks when a server render throws

143
packages/svelte/src/internal/server/renderer.js
Unescape View File

@ -44,6 +44,12 @@ export class Renderer {
*/
#on_destroy = undefined;
/**
* Whether the `onDestroy` callbacks of this renderer tree have been run.
* @type {boolean}
*/
#destroyed = false;
/**
* Whether this renderer is a component body.
* @type {boolean}
@ -316,8 +322,11 @@ export class Renderer {
*/
component(fn, component_fn) {
push(component_fn);
const child = this.child(fn);
child.#is_component_body = true;
// mark before running so `onDestroy` callbacks are still collected if `fn` throws
this.child((renderer) => {
renderer.#is_component_body = true;
return fn(renderer);
});
pop();
}
@ -624,6 +633,75 @@ export class Renderer {
}
}
/**
* Runs the `onDestroy` callbacks of this renderer tree at most once,
* whether the render succeeded or failed.
*/
#run_on_destroy() {
if (this.#destroyed) return;
this.#destroyed = true;
for (const cleanup of this.#collect_on_destroy()) {
cleanup();
}
}
/**
* Waits until every promise in the tree has settled, including promises created
* while waiting. This makes `#collect_on_destroy` safe to call after a failed
* async render, where siblings of the rejected renderer are still in flight.
*/
async #settle() {
/** @type {Set<Promise<void>>} */
const seen = new Set();
/** @type {Promise<void>[]} */
let pending;
do {
pending = [];
this.#collect_pending(seen, pending);
await Promise.allSettled(pending);
} while (pending.length > 0);
}
/**
* @param {Set<Promise<void>>} seen
* @param {Promise<void>[]} pending
*/
#collect_pending(seen, pending) {
if (this.promise !== undefined && !seen.has(this.promise)) {
seen.add(this.promise);
pending.push(this.promise);
}
for (const child of this.#out) {
if (typeof child !== 'string') {
child.#collect_pending(seen, pending);
}
}
}
/**
* @param {'sync' | 'async'} mode
* @param {{ idPrefix?: string; csp?: Csp; transformError?: (error: unknown) => unknown }} options
* @returns {Renderer}
*/
static #create(mode, options) {
if (options.idPrefix?.includes('--')) {
e.invalid_id_prefix();
}
return new Renderer(
new SSRState(
mode,
options.idPrefix ? options.idPrefix + '-' : '',
options.csp,
options.transformError
)
);
}
/**
* Render a component. Throws if any of the children are performing asynchronous work.
*
@ -634,11 +712,23 @@ export class Renderer {
*/
static #render(component, options) {
var previous_context = ssr_context;
const renderer = Renderer.#create('sync', options);
try {
const renderer = Renderer.#open_render('sync', component, options);
Renderer.#open_render(renderer, component, options);
const content = renderer.#collect_content();
return Renderer.#close_render(content, renderer);
const result = Renderer.#close_render(content, renderer);
renderer.#run_on_destroy();
return result;
} catch (error) {
try {
renderer.#run_on_destroy();
} catch {
// a throwing cleanup must not mask the error that failed the render
}
throw error;
} finally {
abort();
set_ssr_context(previous_context);
@ -655,15 +745,31 @@ export class Renderer {
*/
static async #render_async(component, options) {
const previous_context = ssr_context;
const renderer = Renderer.#create('async', options);
try {
const renderer = Renderer.#open_render('async', component, options);
Renderer.#open_render(renderer, component, options);
const content = await renderer.#collect_content_async();
const hydratables = await renderer.#collect_hydratables();
if (hydratables !== null) {
content.head = hydratables + content.head;
}
return Renderer.#close_render(content, renderer);
const result = Renderer.#close_render(content, renderer);
renderer.#run_on_destroy();
return result;
} catch (error) {
// in-flight siblings of the rejected renderer must finish initialising
// before their cleanup runs, and may register more callbacks after resuming
await renderer.#settle();
try {
renderer.#run_on_destroy();
} catch {
// a throwing cleanup must not mask the error that failed the render
}
throw error;
} finally {
set_ssr_context(previous_context);
abort();
@ -758,28 +864,15 @@ export class Renderer {
/**
* @template {Record<string, any>} Props
* @param {'sync' | 'async'} mode
* @param {Renderer} renderer
* @param {import('svelte').Component<Props>} component
* @param {{ props?: Omit<Props, '$$slots' | '$$events'>; context?: Map<any, any>; idPrefix?: string; csp?: Csp; transformError?: (error: unknown) => unknown }} options
* @returns {Renderer}
* @returns {void}
*/
static #open_render(mode, component, options) {
if (options.idPrefix?.includes('--')) {
e.invalid_id_prefix();
}
static #open_render(renderer, component, options) {
var previous_context = ssr_context;
try {
const renderer = new Renderer(
new SSRState(
mode,
options.idPrefix ? options.idPrefix + '-' : '',
options.csp,
options.transformError
)
);
/** @type {SSRContext} */
const context = { p: null, c: options.context ?? null, r: renderer };
set_ssr_context(context);
@ -788,8 +881,6 @@ export class Renderer {
// @ts-expect-error
component(renderer, options.props ?? {});
renderer.push(BLOCK_CLOSE);
return renderer;
} finally {
set_ssr_context(previous_context);
}
@ -801,10 +892,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;

89
packages/svelte/src/internal/server/renderer.test.ts
Unescape View File

@ -466,4 +466,93 @@ describe('async', () => {
await Renderer.render(component as unknown as Component);
expect(destroyed).toEqual(['c', 'e', 'a', 'b', 'b*', 'd']);
});
test('on_destroy callbacks run when a sync render throws', () => {
const destroyed: string[] = [];
const component = (renderer: Renderer) => {
renderer.component((renderer) => {
renderer.on_destroy(() => destroyed.push('a'));
renderer.child(() => {
throw new Error('boom');
});
});
};
expect(() => Renderer.render(component as unknown as Component).body).toThrow('boom');
expect(destroyed).toEqual(['a']);
});
test('on_destroy callbacks run when an async render rejects', async () => {
const destroyed: string[] = [];
const component = (renderer: Renderer) => {
renderer.component((renderer) => {
renderer.on_destroy(() => destroyed.push('a'));
renderer.child(async () => {
await Promise.resolve();
throw new Error('boom');
});
});
};
await expect(Renderer.render(component as unknown as Component)).rejects.toThrow('boom');
expect(destroyed).toEqual(['a']);
});
test('on_destroy waits for in-flight renderers when an async render rejects', async () => {
const events: string[] = [];
let initialised = false;
const component = (renderer: Renderer) => {
renderer.component((renderer) => {
// rejects while the sibling component below is still in flight
renderer.child(async () => {
await Promise.resolve();
throw new Error('boom');
});
renderer.component((renderer) => {
renderer.on_destroy(() => events.push(`before-await (initialised: ${initialised})`));
renderer.child(async () => {
await new Promise((f) => setTimeout(f, 10));
initialised = true;
renderer.on_destroy(() => events.push('after-await'));
});
});
});
};
await expect(Renderer.render(component as unknown as Component)).rejects.toThrow('boom');
expect(events).toEqual(['before-await (initialised: true)', 'after-await']);
});
test('a throwing on_destroy callback does not mask a sync render error', () => {
const component = (renderer: Renderer) => {
renderer.component((renderer) => {
renderer.on_destroy(() => {
throw new Error('cleanup failed');
});
renderer.child(() => {
throw new Error('boom');
});
});
};
expect(() => Renderer.render(component as unknown as Component).body).toThrow('boom');
});
test('a throwing on_destroy callback does not mask an async render error', async () => {
const component = (renderer: Renderer) => {
renderer.component((renderer) => {
renderer.on_destroy(() => {
throw new Error('cleanup failed');
});
renderer.child(async () => {
await Promise.resolve();
throw new Error('boom');
});
});
};
await expect(Renderer.render(component as unknown as Component)).rejects.toThrow('boom');
});
});

Write Preview
Loading…
Cancel
Save