diff --git a/.changeset/requested-outside-remote-request.md b/.changeset/requested-outside-remote-request.md new file mode 100644 index 000000000000..2cf498f3e657 --- /dev/null +++ b/.changeset/requested-outside-remote-request.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: make `requested()` work in commands called from form actions and endpoints diff --git a/.changeset/set-headers-after-response.md b/.changeset/set-headers-after-response.md new file mode 100644 index 000000000000..1da57b64cd37 --- /dev/null +++ b/.changeset/set-headers-after-response.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: throw when `setHeaders` is called after the response has been generated, whichever copy of the event it is called on diff --git a/packages/kit/src/exports/hooks/sequence.js b/packages/kit/src/exports/hooks/sequence.js index 8dd0b546c84e..5b39191fc7e9 100644 --- a/packages/kit/src/exports/hooks/sequence.js +++ b/packages/kit/src/exports/hooks/sequence.js @@ -1,7 +1,7 @@ -/** @import { RequestEvent } from '@sveltejs/kit' */ +/** @import { RequestEvent as Interface } from '@sveltejs/kit' */ /** @import { Handle, ResolveOptions } from '@sveltejs/kit/hooks' */ import { - merge_tracing, + RequestEvent, get_request_store, record_span, with_request_store @@ -90,7 +90,7 @@ export function sequence(...handlers) { /** * @param {number} i - * @param {RequestEvent} event + * @param {Interface} event * @param {ResolveOptions | undefined} parent_options * @returns {Promise} */ @@ -101,7 +101,8 @@ export function sequence(...handlers) { name: `sveltekit.handle.sequenced.${handle.name ? handle.name : i}`, attributes: {}, fn: async (current) => { - const traced_event = merge_tracing(event, current); + const traced_event = RequestEvent.from(event, current); + return await with_request_store({ event: traced_event, state }, () => handle({ event: traced_event, diff --git a/packages/kit/src/exports/hooks/sequence.spec.js b/packages/kit/src/exports/hooks/sequence.spec.js index b9292e13f1fe..ba0d78989ca2 100644 --- a/packages/kit/src/exports/hooks/sequence.spec.js +++ b/packages/kit/src/exports/hooks/sequence.spec.js @@ -17,7 +17,7 @@ vi.mock(import('@sveltejs/kit/internal/server'), async (actualPromise) => { return { ...actual, get_request_store: () => ({ - event: dummy_event, + event: /** @type {any} */ (dummy_event), state: /** @type {RequestState} */ (/** @type {unknown} */ ({})) }) }; diff --git a/packages/kit/src/exports/internal/server/event.js b/packages/kit/src/exports/internal/server/event.js index 3916562a8b00..d289ca552ed4 100644 --- a/packages/kit/src/exports/internal/server/event.js +++ b/packages/kit/src/exports/internal/server/event.js @@ -1,8 +1,231 @@ -/** @import { RequestEvent } from '@sveltejs/kit' */ +/** @import { Cookies, RequestEvent as Interface } from '@sveltejs/kit' */ +/** @import { Span } from '@opentelemetry/api' */ /** @import { RequestStore } from 'types' */ /** @import { AsyncLocalStorage } from 'node:async_hooks' */ +import { DEV } from 'esm-env'; import { IN_WEBCONTAINER } from '../../../constants.js'; +/** The kinds of code an event gets handed to, and the groups the runtime asks about */ +export const QUERY = 1; +export const PRERENDER = 2; +export const FORM = 4; +export const COMMAND = 8; +export const RENDER = 16; +const REMOTE = QUERY | PRERENDER | FORM | COMMAND; + +/** The kinds on the stack, kept under a symbol so it is not part of the public shape */ +export const CONTEXT = Symbol('sveltekit.context'); + +/** @type {Interface['setHeaders']} */ +function forbid_set_headers() { + throw new Error('setHeaders is not allowed in remote functions'); +} + +/** + * What remote functions may do with cookies + * @implements {Cookies} + */ +class RemoteCookies { + #cookies; + #flags; + + /** + * @param {Cookies} cookies + * @param {number} flags the kinds on the stack + */ + constructor(cookies, flags) { + this.#cookies = cookies; + this.#flags = flags; + } + + /** + * @param {'set' | 'delete'} verb + * @param {import('cookie').SerializeOptions} [opts] + */ + #check(verb, opts) { + if (this.#flags & (QUERY | PRERENDER)) { + throw new Error(`Cannot ${verb} cookies in \`query\` or \`prerender\` functions`); + } + if (opts?.path && !opts.path.startsWith('/')) { + throw new Error('Cookies in remote functions must have an absolute path'); + } + } + + /** @type {Cookies['get']} */ + get(name, opts) { + return this.#cookies.get(name, opts); + } + + /** @type {Cookies['getAll']} */ + getAll(opts) { + return this.#cookies.getAll(opts); + } + + /** @type {Cookies['serialize']} */ + serialize(name, value, opts) { + return this.#cookies.serialize(name, value, opts); + } + + /** @type {Cookies['parse']} */ + parse(header, opts) { + return this.#cookies.parse(header, opts); + } + + /** @type {Cookies['set']} */ + set(name, value, opts) { + this.#check('set', opts); + return this.#cookies.set(name, value, opts); + } + + /** @type {Cookies['delete']} */ + delete(name, opts) { + this.#check('delete', opts); + return this.#cookies.delete(name, opts); + } +} + +/** + * The event as a class, so that a view for a kind of code is a clone with a fixed field list + * rather than a copy of whatever the source enumerates + * @implements {Interface} + */ +export class RequestEvent { + /** @type {number} */ + [CONTEXT]; + + /** + * @param {Interface} source + * @param {number} flags + */ + constructor(source, flags) { + this.cookies = source.cookies; + this.fetch = source.fetch; + this.getClientAddress = source.getClientAddress; + this.locals = source.locals; + this.platform = source.platform; + this.request = source.request; + this.setHeaders = source.setHeaders; + this.url = source.url; + this.params = source.params; + this.route = source.route; + this.isDataRequest = source.isDataRequest; + this.isSubRequest = source.isSubRequest; + this.isRemoteRequest = source.isRemoteRequest; + this.tracing = source.tracing; + this[CONTEXT] = flags; + } + + /** + * A traced copy of the event a `handle` hook passes on, which it may have built by hand + * @param {Interface} event + * @param {Span} current + * @returns {RequestEvent} + */ + static from(event, current) { + const view = new RequestEvent(event, 0); + view.tracing = { ...event.tracing, current }; + return view; + } + + /** Inside a `query` function, however deep */ + get in_query() { + return (this[CONTEXT] & QUERY) !== 0; + } + + /** Inside a `prerender` function, however deep */ + get in_prerender() { + return (this[CONTEXT] & PRERENDER) !== 0; + } + + /** Inside a `form` or `command` function */ + get in_mutation() { + return (this[CONTEXT] & (FORM | COMMAND)) !== 0; + } + + /** Inside any remote function */ + get in_remote() { + return (this[CONTEXT] & REMOTE) !== 0; + } + + /** While the page renders */ + get in_render() { + return (this[CONTEXT] & RENDER) !== 0; + } + + /** + * The only way to copy an event: a view for the given kind of code, minus what that kind + * may not do, with the kinds already on the stack carried along + * @param {number} [kind] + * @returns {RequestEvent} + */ + clone(kind = 0) { + const flags = this[CONTEXT] | kind; + const view = + flags & QUERY + ? /** @type {RequestEvent} */ (new QueryEvent(this, flags)) + : new RequestEvent(this, flags); + + if (kind & REMOTE) { + view.cookies = new RemoteCookies(this.cookies, flags); + view.setHeaders = forbid_set_headers; + } + + return view; + } + + /** + * @param {Span} current + * @returns {RequestEvent} + */ + traced(current) { + const view = this.clone(); + view.tracing = { ...this.tracing, current }; + return view; + } +} + +/** + * A query may not read the page, so a query view never copies it and reads throw. It copies + * its own field list instead of extending `RequestEvent`, which would run the parent + * constructor and hand its stores a second shape + * @implements {Omit} + */ +class QueryEvent { + /** @type {number} */ + [CONTEXT]; + + /** + * @param {Interface} source + * @param {number} flags + */ + constructor(source, flags) { + this.cookies = source.cookies; + this.fetch = source.fetch; + this.getClientAddress = source.getClientAddress; + this.locals = source.locals; + this.platform = source.platform; + this.request = source.request; + this.setHeaders = source.setHeaders; + this.isDataRequest = source.isDataRequest; + this.isSubRequest = source.isSubRequest; + this.isRemoteRequest = source.isRemoteRequest; + this.tracing = source.tracing; + this[CONTEXT] = flags; + } +} + +Object.setPrototypeOf(QueryEvent.prototype, RequestEvent.prototype); + +for (const property of ['url', 'params', 'route']) { + Object.defineProperty(QueryEvent.prototype, property, { + get() { + throw new Error( + `Cannot access event.${property} in a query. Pass the value as an argument to the query instead` + ); + } + }); +} + /** @type {RequestStore | null} */ let sync_store = null; @@ -23,7 +246,7 @@ import('node:async_hooks') * In environments without [`AsyncLocalStorage`](https://nodejs.org/api/async_context.html#class-asynclocalstorage), this must be called synchronously (i.e. not after an `await`). * @since 2.20.0 * - * @returns {RequestEvent} + * @returns {Interface} */ export function getRequestEvent() { const event = try_get_request_store()?.event; @@ -71,6 +294,10 @@ export function try_get_request_store() { * @param {() => T} fn */ export function with_request_store(store, fn) { + if (DEV && store && !(store.event instanceof RequestEvent)) { + throw new Error('The request store only holds events made by `RequestEvent`, never a copy'); + } + try { sync_store = store; return als ? als.run(store, fn) : fn(); diff --git a/packages/kit/src/exports/internal/server/event.spec.js b/packages/kit/src/exports/internal/server/event.spec.js new file mode 100644 index 000000000000..fa4620a4313b --- /dev/null +++ b/packages/kit/src/exports/internal/server/event.spec.js @@ -0,0 +1,83 @@ +/** @import { RequestEvent as Interface } from '@sveltejs/kit' */ +import { assert, expect, test } from 'vitest'; +import { RequestEvent, CONTEXT, QUERY, COMMAND, RENDER } from './event.js'; + +function root() { + return new RequestEvent( + /** @type {Interface} */ ( + /** @type {unknown} */ ({ + url: new URL('http://localhost/page'), + params: { id: '1' }, + route: { id: '/page' }, + cookies: { set: () => {}, delete: () => {} }, + setHeaders: () => {}, + tracing: { enabled: false } + }) + ), + 0 + ); +} + +test('flags accumulate through nested views', () => { + const event = root().clone(RENDER).clone(QUERY); + + assert.isTrue(event.in_render); + assert.isTrue(event.in_query); + assert.isTrue(event.in_remote); + assert.isFalse(event.in_prerender); + assert.isFalse(event.in_mutation); + assert.isFalse(root().in_render); +}); + +test('a query view throws on access to the page, on every copy', () => { + const query = root().clone(QUERY); + const traced = query.clone(); + + for (const event of [query, traced, traced.clone(QUERY)]) { + for (const property of /** @type {const} */ (['url', 'params', 'route'])) { + expect(() => event[property]).toThrow(`Cannot access event.${property} in a query`); + } + } + + assert.equal(root().clone(COMMAND).url.pathname, '/page'); +}); + +test('a spread of a view is not an event any more', () => { + const copy = { ...root().clone(QUERY) }; + + assert.isUndefined(copy.url); + assert.isFalse(copy instanceof RequestEvent); + assert.isUndefined(/** @type {any} */ (copy).in_query); + expect(() => /** @type {any} */ (copy).clone(QUERY)).toThrow(TypeError); +}); + +test('an event built by hand for `resolve` is adopted as a traced root', () => { + const span = /** @type {any} */ ({}); + const adopted = RequestEvent.from({ ...root() }, span); + + assert.isTrue(adopted instanceof RequestEvent); + assert.isFalse(adopted.in_render); + assert.strictEqual(adopted.tracing.current, span); +}); + +test('views share the request data and own nothing else', () => { + const base = root(); + const event = base.clone(QUERY); + + assert.strictEqual(event.locals, base.locals); + assert.deepEqual(Object.getOwnPropertySymbols(event), [CONTEXT]); + assert.isFalse(Object.hasOwn(event, 'url')); +}); + +test('remote views restrict headers and cookies', () => { + const query = root().clone(QUERY); + const command = root().clone(COMMAND); + + expect(() => query.setHeaders({})).toThrow('setHeaders is not allowed'); + expect(() => query.cookies.set('a', 'b', { path: '/' })).toThrow('Cannot set cookies'); + expect(() => command.cookies.set('a', 'b', { path: 'x' })).toThrow('absolute path'); + command.cookies.set('a', 'b', { path: '/' }); + expect(() => command.clone(QUERY).cookies.set('a', 'b', { path: '/' })).toThrow( + 'Cannot set cookies' + ); +}); diff --git a/packages/kit/src/exports/internal/server/index.js b/packages/kit/src/exports/internal/server/index.js index a48db594f0db..c68599190030 100644 --- a/packages/kit/src/exports/internal/server/index.js +++ b/packages/kit/src/exports/internal/server/index.js @@ -1,4 +1,3 @@ -/** @import { Span } from '@opentelemetry/api' */ import { try_get_request_store } from './event.js'; export function get_origin() { @@ -7,27 +6,17 @@ export function get_origin() { return request && new URL(request.url).origin; } -/** - * @template {{ tracing: { enabled: boolean, root: Span, current: Span } }} T - * @param {T} event_like - * @param {Span} current - * @returns {T} - */ -export function merge_tracing(event_like, current) { - return { - ...event_like, - tracing: { - ...event_like.tracing, - current - } - }; -} - export { with_request_store, getRequestEvent, get_request_store, - try_get_request_store + try_get_request_store, + RequestEvent, + QUERY, + PRERENDER, + FORM, + COMMAND, + RENDER } from './event.js'; export { init_remote_functions } from './remote-functions.js'; diff --git a/packages/kit/src/runtime/app/server/remote/command.js b/packages/kit/src/runtime/app/server/remote/command.js index ddc9fcb9be9e..680ade03dc70 100644 --- a/packages/kit/src/runtime/app/server/remote/command.js +++ b/packages/kit/src/runtime/app/server/remote/command.js @@ -1,7 +1,7 @@ /** @import { RemoteCommand } from '$app/server' */ /** @import { MaybePromise, RemoteCommandInternals } from 'types' */ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ -import { get_request_store } from '@sveltejs/kit/internal/server'; +import { get_request_store, COMMAND } from '@sveltejs/kit/internal/server'; import { create_validator, run_remote_function } from './shared.js'; import { MUTATIVE_METHODS } from '../../../../constants.js'; @@ -64,26 +64,22 @@ export function command(validate_or_fn, maybe_fn) { /** @type {RemoteCommand & { __: RemoteCommandInternals }} */ const wrapper = (arg) => { const { event, state } = get_request_store(); + const nested = event.in_query || event.in_prerender; - if ( - !MUTATIVE_METHODS.includes(event.request.method) || - state.is_in_remote_query || - state.is_in_remote_prerender - ) { - const violation = - state.is_in_remote_query || state.is_in_remote_prerender - ? `inside a query or prerender function` - : `from a ${event.request.method} handler`; + if (nested || !MUTATIVE_METHODS.includes(event.request.method)) { + const violation = nested + ? `inside a query or prerender function` + : `from a ${event.request.method} handler`; throw new Error(`Cannot call a command (${__.name}) ${violation}`); } - if (state.is_in_render) { + if (event.in_render) { throw new Error(`Cannot call a command (${__.name}) during server-side rendering`); } const promise = Promise.resolve( - run_remote_function(event, state, true, () => validate(arg), fn) + run_remote_function(event, state, COMMAND, () => validate(arg), fn) ); // @ts-expect-error diff --git a/packages/kit/src/runtime/app/server/remote/form.js b/packages/kit/src/runtime/app/server/remote/form.js index 33efa448ddd6..232397734007 100644 --- a/packages/kit/src/runtime/app/server/remote/form.js +++ b/packages/kit/src/runtime/app/server/remote/form.js @@ -1,7 +1,7 @@ /** @import { RemoteFormInput, RemoteForm, RemoteFormInvalidField } from '$app/server' */ /** @import { InternalRemoteFormIssue, MaybePromise, HasNonOptionalBoolean, RemoteFormInternals } from 'types' */ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ -import { get_request_store } from '@sveltejs/kit/internal/server'; +import { FORM, get_request_store } from '@sveltejs/kit/internal/server'; import { create_field_proxy, split_path, @@ -118,7 +118,7 @@ export function form(validate_or_fn, maybe_fn) { output.result = await run_remote_function( event, state, - true, + FORM, () => data, (data) => (!maybe_fn ? fn() : fn(data, issue)) ); diff --git a/packages/kit/src/runtime/app/server/remote/prerender.js b/packages/kit/src/runtime/app/server/remote/prerender.js index b7aa91f10ad3..a8b5d07504e3 100644 --- a/packages/kit/src/runtime/app/server/remote/prerender.js +++ b/packages/kit/src/runtime/app/server/remote/prerender.js @@ -2,7 +2,7 @@ /** @import { RemoteFunctionResponse, RemotePrerenderInputsGenerator, RemotePrerenderInternals, MaybePromise } from 'types' */ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ import { HandledHttpError } from '@sveltejs/kit/internal'; -import { get_request_store } from '@sveltejs/kit/internal/server'; +import { get_request_store, PRERENDER } from '@sveltejs/kit/internal/server'; import { stringify_remote_arg } from '../../../shared.js'; import { parse, stringify } from '#app/internal/transport'; import { noop } from '../../../../utils/functions.js'; @@ -90,7 +90,7 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) { // implicit lookup, so that the result is inlined into the page payload (`data.p`) // and the client doesn't need to fetch it again upon hydration /** @type {Promise & Partial>} */ - const promise = get_response(__, payload, state, async () => { + const promise = get_response(__, payload, event, state, async () => { const id = __.id; const url = `${base}/${app_dir}/remote/${id}${payload ? `/${payload}` : ''}`; @@ -126,13 +126,7 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) { return /** @type {Promise} */ (state.prerendering.remote_responses.get(url)); } - const promise = run_remote_function( - event, - { ...state, is_in_remote_prerender: true }, - false, - () => validate(arg), - fn - ); + const promise = run_remote_function(event, state, PRERENDER, () => validate(arg), fn); if (state.prerendering) { state.prerendering.remote_responses.set(url, promise); diff --git a/packages/kit/src/runtime/app/server/remote/prerender.spec.js b/packages/kit/src/runtime/app/server/remote/prerender.spec.js index 73b418882b7c..eeeca72eb077 100644 --- a/packages/kit/src/runtime/app/server/remote/prerender.spec.js +++ b/packages/kit/src/runtime/app/server/remote/prerender.spec.js @@ -1,8 +1,8 @@ -/** @import { RequestEvent } from '@sveltejs/kit' */ /** @import { RequestState } from 'types' */ import { expect, test, vi } from 'vitest'; import { HandledHttpError, ValidationError } from '@sveltejs/kit/internal'; import { prerender } from './prerender.js'; +import { RequestEvent } from '@sveltejs/kit/internal/server'; import { init_transport, stringify } from '#app/internal/transport'; init_transport({}); @@ -33,18 +33,20 @@ function setup(fetch_impl) { /** @type {any} */ (wrapper).__.id = 'hash/fn'; store.current = { - event: /** @type {RequestEvent} */ ( - /** @type {unknown} */ ({ - request: { url: 'http://localhost/' }, - isRemoteRequest: false, - cookies: {} - }) + event: new RequestEvent( + /** @type {import('@sveltejs/kit').RequestEvent} */ ( + /** @type {unknown} */ ({ + request: { url: 'http://localhost/' }, + isRemoteRequest: false, + cookies: {} + }) + ), + 0 ), state: /** @type {RequestState} */ ( /** @type {unknown} */ ({ remote: {}, - prerendering: undefined, - is_in_remote_query: false + prerendering: undefined }) ) }; diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index 7eb5bf20490a..b97add126124 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -1,8 +1,8 @@ /** @import { RemoteLiveQuery, RemoteLiveQueryFunction, RemoteQuery, RemoteQueryFunction } from '$app/server' */ -/** @import { RequestEvent } from '@sveltejs/kit' */ +/** @import { RequestEvent } from '@sveltejs/kit/internal/server' */ /** @import { RemoteInternals, MaybePromise, RequestState, RemoteQueryLiveInternals, RemoteQueryBatchInternals, RemoteQueryInternals, RemoteLiveQueryUserFunctionReturnType } from 'types' */ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ -import { get_request_store } from '@sveltejs/kit/internal/server'; +import { get_request_store, QUERY } from '@sveltejs/kit/internal/server'; import { create_remote_key, stringify_remote_arg } from '../../../shared.js'; import { prerendering } from '#app/env/server'; import { @@ -79,13 +79,7 @@ export function query(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); return create_query_resource(__, payload, event, state, () => - run_remote_function( - event, - { ...state, is_in_remote_query: true }, - false, - () => validated_arg, - fn - ) + run_remote_function(event, state, QUERY, () => validated_arg, fn) ); } }; @@ -102,13 +96,7 @@ export function query(validate_or_fn, maybe_fn) { const payload = stringify_remote_arg(arg); return create_query_resource(__, payload, event, state, () => - run_remote_function( - event, - { ...state, is_in_remote_query: true }, - false, - () => validate(arg), - fn - ) + run_remote_function(event, state, QUERY, () => validate(arg), fn) ); }; @@ -164,14 +152,7 @@ function live(validate_or_fn, maybe_fn) { * @param {any} get_input */ const run = (event, state, get_input) => - run_remote_generator( - event, - { ...state, is_in_remote_query: true }, - false, - get_input, - fn, - __.name - ); + run_remote_generator(event, state, QUERY, get_input, fn, __.name); /** @type {RemoteQueryLiveInternals} */ const __ = { @@ -292,8 +273,8 @@ function batch(validate_or_fn, maybe_fn) { try { return await run_remote_function( event, - { ...state, is_in_remote_query: true }, - false, + state, + QUERY, async () => Promise.all(entries.map((entry) => entry.get_validated())), async (input) => { const get_result = await fn(input); @@ -335,8 +316,8 @@ function batch(validate_or_fn, maybe_fn) { return run_remote_function( event, - { ...state, is_in_remote_query: true }, - false, + state, + QUERY, async () => Promise.all(args.map(validate)), async (/** @type {any[]} */ input) => { const get_result = await fn(input); @@ -405,9 +386,9 @@ export function refresh(event, state, internals, payload, fn) { return; } - if (!event.isRemoteRequest && state.is_in_remote_form_or_command) { - // ...or this is a no-JS (native) form submission, where the page re-renders - // anyway so there's no live client cache to apply a single-flight update to. + if (!event.isRemoteRequest && event.in_mutation) { + // ...or the mutation runs outside a remote request (a no-JS form submission, or a + // command called from an action or endpoint), so there is no client cache to update. return; } @@ -437,14 +418,14 @@ function create_query_resource(__, payload, event, state, fn) { let promise = null; const get_promise = () => { - return (promise ??= get_response(__, payload, state, fn)); + return (promise ??= get_response(__, payload, event, state, fn)); }; const populate_hydratable = () => { // accessing data properties needs to kick off the work // so that it gets seeded in the hydration cache // and becomes available on the client - if (__.id && state.is_in_render) { + if (__.id && event.in_render) { // swallow rejections so they don't crash the server — the error is // serialized into the response and surfaced on the client instead get_promise().catch(noop); @@ -524,11 +505,11 @@ function create_live_query_resource(__, payload, event, state, get_generator) { }; const get_promise = () => { - return (promise ??= get_response(__, payload, state, get_first_value)); + return (promise ??= get_response(__, payload, event, state, get_first_value)); }; const populate_hydratable = () => { - if (__.id && state.is_in_render) { + if (__.id && event.in_render) { // swallow rejections so they don't crash the server — the error is // serialized into the response and surfaced on the client instead get_promise().catch(noop); diff --git a/packages/kit/src/runtime/app/server/remote/requested.js b/packages/kit/src/runtime/app/server/remote/requested.js index 29a1f3075b07..dc343b16c191 100644 --- a/packages/kit/src/runtime/app/server/remote/requested.js +++ b/packages/kit/src/runtime/app/server/remote/requested.js @@ -134,11 +134,7 @@ export function requested(query, limit) { ignored.add(create_remote_key(__.id, payload)); }; - // note: don't initialize these maps here -- they will be initialized by the - // command/form wrapper when we enter them, and if we initialize them here - // we will enable requested(...) in contexts where it shouldn't be allowed, - // such as load functions or other server functions - if (!state.is_in_remote_form_or_command) { + if (!event.in_mutation) { throw new Error( 'requested(...) can only be called in the context of a command/form remote function' ); diff --git a/packages/kit/src/runtime/app/server/remote/shared.js b/packages/kit/src/runtime/app/server/remote/shared.js index 6b995bc22afc..f3b4eec105db 100644 --- a/packages/kit/src/runtime/app/server/remote/shared.js +++ b/packages/kit/src/runtime/app/server/remote/shared.js @@ -1,5 +1,5 @@ -/** @import { RequestEvent } from '@sveltejs/kit' */ -/** @import { MaybePromise, RequestState, RemoteInternals, RequestStore, RemoteLiveQueryUserFunctionReturnType } from 'types' */ +/** @import { RequestEvent } from '@sveltejs/kit/internal/server' */ +/** @import { MaybePromise, RequestState, RemoteInternals, RemoteLiveQueryUserFunctionReturnType } from 'types' */ import { error } from '@sveltejs/kit'; import { ValidationError } from '@sveltejs/kit/internal'; import { with_request_store } from '@sveltejs/kit/internal/server'; @@ -54,18 +54,19 @@ export function create_validator(validate_or_fn, maybe_fn) { * @template {MaybePromise} T * @param {RemoteInternals} internals * @param {string} payload — the stringified raw argument (i.e. the cache key the client will use) + * @param {RequestEvent} event * @param {RequestState} state * @param {() => Promise} get_result * @returns {Promise} */ -export async function get_response(internals, payload, state, get_result) { +export async function get_response(internals, payload, event, state, get_result) { // wait a beat, in case `myQuery().set(...)` or `myQuery().refresh()` is immediately called // eslint-disable-next-line @typescript-eslint/await-thenable await 0; const cache = get_cache(internals, state); - if (!state.is_in_remote_query) { + if (!event.in_query) { // if this is a top-level (not nested) `await myQuery()`, include it in the serialized response get_implicit_lookup(internals, state)[payload] = get_result; } @@ -73,80 +74,17 @@ export async function get_response(internals, payload, state, get_result) { return (cache[payload] ??= get_result()); } -/** - * @param {RequestEvent} event - * @param {RequestState} state - * @param {boolean} allow_cookies - * @returns {RequestStore} - */ -function derive_remote_function_event(event, state, allow_cookies) { - /** @type {RequestEvent} */ - const derived = { - ...event, - setHeaders: () => { - throw new Error('setHeaders is not allowed in remote functions'); - }, - cookies: { - ...event.cookies, - set: (name, value, opts) => { - if (!allow_cookies) { - throw new Error('Cannot set cookies in `query` or `prerender` functions'); - } - - if (opts?.path && !opts.path.startsWith('/')) { - throw new Error('Cookies set in remote functions must have an absolute path'); - } - - return event.cookies.set(name, value, opts); - }, - delete: (name, opts) => { - if (!allow_cookies) { - throw new Error('Cannot delete cookies in `query` or `prerender` functions'); - } - - if (opts?.path && !opts.path.startsWith('/')) { - throw new Error('Cookies deleted in remote functions must have an absolute path'); - } - - return event.cookies.delete(name, opts); - } - } - }; - - if (state.is_in_remote_query) { - for (const property of ['url', 'params', 'route']) { - // non-enumerable so spreading for a nested derivation doesn't invoke the getter - Object.defineProperty(derived, property, { - enumerable: false, - get() { - throw new Error( - `Cannot access event.${property} in a query. Pass the value as an argument to the query instead` - ); - } - }); - } - } - - return { - event: derived, - state: { - ...state, - is_in_remote_function: true - } - }; -} - /** * Like `with_event` but removes things from `event` you cannot see/call in remote functions, such as `setHeaders`. * @template T * @param {RequestEvent} event * @param {RequestState} state - * @param {boolean} allow_cookies + * @param {number} kind * @param {() => any} get_input * @param {(arg?: any) => T} fn */ -export async function run_remote_function(event, state, allow_cookies, get_input, fn) { - const store = derive_remote_function_event(event, state, allow_cookies); +export async function run_remote_function(event, state, kind, get_input, fn) { + const store = { event: event.clone(kind), state }; // In two parts, each with_event, so that runtimes without async local storage can still get the event at the start of the function const input = await with_request_store(store, get_input); @@ -158,13 +96,13 @@ export async function run_remote_function(event, state, allow_cookies, get_input * @template T * @param {RequestEvent} event * @param {RequestState} state - * @param {boolean} allow_cookies + * @param {number} kind * @param {() => any} get_input * @param {(arg?: any) => RemoteLiveQueryUserFunctionReturnType} fn * @param {string} name */ -export async function* run_remote_generator(event, state, allow_cookies, get_input, fn, name) { - const store = derive_remote_function_event(event, state, allow_cookies); +export async function* run_remote_generator(event, state, kind, get_input, fn, name) { + const store = { event: event.clone(kind), state }; // In two parts, each with_event, so that runtimes without async local storage can still get the event at the start of the function / calls to next const input = await with_request_store(store, get_input); diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js index 85052e10b0b2..d2138671fdcc 100644 --- a/packages/kit/src/runtime/server/data/index.js +++ b/packages/kit/src/runtime/server/data/index.js @@ -11,7 +11,7 @@ import { with_version_header } from '../utils.js'; import { manifest } from '../internal.js'; /** - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {import('types').RequestState} state * @param {{ page: Pick | null }} route * @param {boolean[] | undefined} invalidated_data_nodes @@ -33,7 +33,8 @@ export async function render_data(event, state, route, invalidated_data_nodes, t const url = new URL(event.url); url.pathname = normalize_path(url.pathname, trailing_slash); - const new_event = { ...event, url }; + const new_event = event.clone(); + new_event.url = url; const functions = node_ids.map((n, i) => { return once(async () => { diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 30539991a69e..4aedccb024e1 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -5,7 +5,7 @@ import { negotiate } from '../../utils/http.js'; import { method_not_allowed } from './utils.js'; /** - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {import('types').RequestState} state * @param {import('types').SSREndpoint} mod * @returns {Promise} @@ -47,9 +47,7 @@ export async function render_endpoint(event, state, mod) { } try { - const response = await with_request_store({ event, state }, () => - handler(/** @type {import('@sveltejs/kit').RequestEvent>} */ (event)) - ); + const response = await with_request_store({ event, state }, () => handler(event)); if (!(response instanceof Response)) { throw new Error( diff --git a/packages/kit/src/runtime/server/errors.js b/packages/kit/src/runtime/server/errors.js index b67d7a4c1745..f46db4614278 100644 --- a/packages/kit/src/runtime/server/errors.js +++ b/packages/kit/src/runtime/server/errors.js @@ -10,7 +10,7 @@ import { add_deprecated_handle_error_properties, coalesce_to_error } from '../.. import { fix_stack_trace, hooks } from './internal.js'; /** - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {import('types').RequestState} state * @param {any} error * @returns {App.Error | Promise} @@ -69,7 +69,7 @@ export function handle_error_and_jsonify(event, state, error) { } if (result instanceof Promise) { - if (!__SVELTEKIT_SUPPORTS_ASYNC__ && state.is_in_render) { + if (!__SVELTEKIT_SUPPORTS_ASYNC__ && event.in_render) { console.warn( `To use an async \`handleError\` hook to handle errors that occur during rendering, you must enable \`compilerOptions.experimental.async\` in the SvelteKit plugin of your Vite config. The returned error has been replaced with a generic object` ); diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 6699856221fe..b4a1f19f5b1d 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -1,9 +1,10 @@ -/** @import { RequestEvent, Actions } from '@sveltejs/kit' */ +/** @import { Actions } from '@sveltejs/kit' */ +/** @import { RequestEvent } from '@sveltejs/kit/internal/server' */ /** @import { ActionResult } from '$app/forms' */ /** @import { SSRNode, ServerNode, ServerActionResult } from 'types' */ import { DEV } from 'esm-env'; import { HttpError, Redirect, ActionFailure, SvelteKitError } from '@sveltejs/kit/internal'; -import { with_request_store, merge_tracing, record_span } from '@sveltejs/kit/internal/server'; +import { with_request_store, record_span } from '@sveltejs/kit/internal/server'; import { normalize_error } from '../../../utils/error.js'; import { is_form_content_type, negotiate } from '../../../utils/http.js'; import { with_version_header } from '../utils.js'; @@ -255,7 +256,7 @@ async function call_action(event, state, actions) { 'http.route': event.route.id || 'unknown' }, fn: async (current) => { - const traced_event = merge_tracing(event, current); + const traced_event = event.traced(current); const result = await with_request_store({ event: traced_event, state }, () => action(traced_event) diff --git a/packages/kit/src/runtime/server/page/data_serializer.js b/packages/kit/src/runtime/server/page/data_serializer.js index 286373e89e83..c2aef76ca7a3 100644 --- a/packages/kit/src/runtime/server/page/data_serializer.js +++ b/packages/kit/src/runtime/server/page/data_serializer.js @@ -8,7 +8,7 @@ import { encoders } from '#app/internal/transport'; /** * If the serialized data contains promises, `chunks` will be an * async iterable containing their resolutions - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {import('types').RequestState} state * @returns {import('./types.js').ServerDataSerializer} */ @@ -123,7 +123,7 @@ export function server_data_serializer(event, state) { /** * If the serialized data contains promises, `chunks` will be an * async iterable containing their resolutions - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {import('types').RequestState} state * @returns {import('./types.js').ServerDataSerializerJson} */ diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 136212acdcdb..0cb892d35d5f 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -1,4 +1,4 @@ -/** @import { RequestEvent } from '@sveltejs/kit' */ +/** @import { RequestEvent } from '@sveltejs/kit/internal/server' */ /** @import { PageNodeIndexes, RequestState, RequiredResolveOptions, ServerDataNode, SSRNode } from 'types' */ import { text } from '@sveltejs/kit'; import { Redirect } from '@sveltejs/kit/internal'; diff --git a/packages/kit/src/runtime/server/page/load_data.js b/packages/kit/src/runtime/server/page/load_data.js index 93941f4c6bec..2bce90954f53 100644 --- a/packages/kit/src/runtime/server/page/load_data.js +++ b/packages/kit/src/runtime/server/page/load_data.js @@ -2,7 +2,7 @@ import { DEV } from 'esm-env'; import { noop } from '../../../utils/functions.js'; import { disable_search, make_trackable } from '../../../utils/url.js'; import { fetch_cache_url, validate_depends, validate_load_response } from '../../shared.js'; -import { with_request_store, merge_tracing, record_span } from '@sveltejs/kit/internal/server'; +import { with_request_store, record_span } from '@sveltejs/kit/internal/server'; import { base64_encode } from '../../utils.js'; import { NULL_BODY_STATUS } from '../constants.js'; import { get_node_type } from '../utils.js'; @@ -10,7 +10,7 @@ import { get_node_type } from '../utils.js'; /** * Calls the user's server `load` function. * @param {{ - * event: import('@sveltejs/kit').RequestEvent; + * event: import('@sveltejs/kit/internal/server').RequestEvent; * state: import('types').RequestState; * node: import('types').SSRNode | undefined; * parent: () => Promise>; @@ -80,7 +80,7 @@ export async function load_server_data({ event, state, node, parent }) { 'http.route': event.route.id || 'unknown' }, fn: async (current) => { - const traced_event = merge_tracing(event, current); + const traced_event = event.traced(current); const result = await with_request_store({ event: traced_event, state }, () => load.call(null, { ...traced_event, @@ -191,7 +191,7 @@ export async function load_server_data({ event, state, node, parent }) { /** * Calls the user's `load` function. * @param {{ - * event: import('@sveltejs/kit').RequestEvent; + * event: import('@sveltejs/kit/internal/server').RequestEvent; * state: import('types').RequestState; * fetched: import('./types.js').Fetched[]; * node: import('types').SSRNode | undefined; @@ -229,7 +229,7 @@ export async function load_data({ 'http.route': event.route.id || 'unknown' }, fn: async (current) => { - const traced_event = merge_tracing(event, current); + const traced_event = event.traced(current); return await with_request_store({ event: traced_event, state }, () => load.call(null, { diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index d38fac024320..ce20822b0c8d 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -22,7 +22,7 @@ import { add_resolution_suffix, route_id_resolution_pathname } from '../../pathname.js'; -import { try_get_request_store, with_request_store } from '@sveltejs/kit/internal/server'; +import { try_get_request_store, with_request_store, RENDER } from '@sveltejs/kit/internal/server'; import { stream_text } from '../../utils.js'; import { count_non_ssi_comments } from '../utils.js'; import { handle_error_and_jsonify } from '../errors.js'; @@ -45,7 +45,7 @@ import { options } from '/server.js'; * page_config: { ssr: boolean; csr: boolean }; * status: number; * error: App.Error | null; - * event: import('@sveltejs/kit').RequestEvent; + * event: import('@sveltejs/kit/internal/server').RequestEvent; * state: import('types').RequestState; * resolve_opts: import('types').RequiredResolveOptions; * action_result?: import('types').ServerActionResult; @@ -181,7 +181,7 @@ export async function render_response({ props.page.data = data; - const render_state = { ...state, is_in_render: true }; + const render_event = event.clone(RENDER); const render_opts = { context: new Map([ @@ -199,7 +199,7 @@ export async function render_response({ throw e; } - const handled = handle_error_and_jsonify(event, render_state, e); + const handled = handle_error_and_jsonify(render_event, state, e); // TODO 4.0 make this an async function and await `handled` if (handled instanceof Promise) { @@ -230,7 +230,7 @@ export async function render_response({ throw new Error( `Cannot call \`fetch\` eagerly during server-side rendering with relative URL (${info}) — put your \`fetch\` calls inside \`onMount\` or a \`load\` function instead` ); - } else if (!warned && !try_get_request_store()?.state.is_in_remote_function) { + } else if (!warned && !try_get_request_store()?.event.in_remote) { console.warn( 'Avoid calling `fetch` eagerly during server-side rendering — put your `fetch` calls inside `onMount` or a `load` function instead' ); @@ -241,7 +241,7 @@ export async function render_response({ }; } - rendered = await with_request_store({ event, state: render_state }, async () => { + rendered = await with_request_store({ event: render_event, state }, async () => { return render(Root, { ...render_opts, props }); }); diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js index 96bdc540bc6b..6769a1d5729e 100644 --- a/packages/kit/src/runtime/server/page/respond_with_error.js +++ b/packages/kit/src/runtime/server/page/respond_with_error.js @@ -17,7 +17,7 @@ import { escape_html } from '../../../utils/escape.js'; /** * @param {{ - * event: import('@sveltejs/kit').RequestEvent; + * event: import('@sveltejs/kit/internal/server').RequestEvent; * state: import('types').RequestState; * error: unknown; * resolve_opts: import('types').RequiredResolveOptions; @@ -131,7 +131,7 @@ export function static_error_page(status, message) { } /** - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {import('types').RequestState} state * @param {unknown} error */ diff --git a/packages/kit/src/runtime/server/remote-functions.js b/packages/kit/src/runtime/server/remote-functions.js index 5c6eadee66ed..669b86d0abee 100644 --- a/packages/kit/src/runtime/server/remote-functions.js +++ b/packages/kit/src/runtime/server/remote-functions.js @@ -1,10 +1,10 @@ -/** @import { RequestEvent } from '@sveltejs/kit' */ +/** @import { RequestEvent } from '@sveltejs/kit/internal/server' */ /** @import { RemoteForm } from '$app/server' */ /** @import { RemoteFormInternals, RemoteFunctionData, RemoteFunctionResponse, RemoteInternals, RequestState, ServerActionResult } from 'types' */ import { error } from '@sveltejs/kit'; import { Redirect, SvelteKitError } from '@sveltejs/kit/internal'; -import { with_request_store, merge_tracing, record_span } from '@sveltejs/kit/internal/server'; +import { with_request_store, record_span } from '@sveltejs/kit/internal/server'; import { app_dir, base } from '#app/paths'; import { is_form_content_type } from '../../utils/http.js'; import { create_remote_key, parse_remote_arg, split_remote_key } from '../shared.js'; @@ -35,12 +35,10 @@ const KEEP_ALIVE_INTERVAL = 30_000; */ export function create_live_query_response(event, state, internals, arg) { const cancellation = new AbortController(); - const live_event = { - ...event, - request: new Request(event.request, { - signal: AbortSignal.any([event.request.signal, cancellation.signal]) - }) - }; + const live_event = event.clone(); + live_event.request = new Request(event.request, { + signal: AbortSignal.any([event.request.signal, cancellation.signal]) + }); const generator = internals.run(live_event, state, arg); @@ -148,7 +146,7 @@ export async function handle_remote_call(event, state, id) { 'sveltekit.remote.call.id': id }, fn: async (current) => { - const traced_event = merge_tracing(event, current); + const traced_event = event.traced(current); const response = await with_request_store({ event: traced_event, state }, () => handle_remote_call_internal(traced_event, state, id) ); @@ -257,10 +255,7 @@ async function handle_remote_call_internal(event, state, id) { } const fn = internals.fn; - data._ = await with_request_store( - { event, state: { ...state, is_in_remote_form_or_command: true } }, - () => fn(input, meta, form_data) - ); + data._ = await with_request_store({ event, state }, () => fn(input, meta, form_data)); if (data._.issues) { // special case — don't serialize refreshes/reconnects @@ -282,10 +277,7 @@ async function handle_remote_call_internal(event, state, id) { state.remote.requested = create_requested_map(refreshes); const arg = parse_remote_arg(payload); - data._ = await with_request_store( - { event, state: { ...state, is_in_remote_form_or_command: true } }, - () => fn(arg) - ); + data._ = await with_request_store({ event, state }, () => fn(arg)); break; } @@ -520,7 +512,7 @@ export async function handle_remote_form_post(event, state, id) { 'sveltekit.remote.form.post.id': id }, fn: (current) => { - const traced_event = merge_tracing(event, current); + const traced_event = event.traced(current); return with_request_store({ event: traced_event, state }, () => handle_remote_form_post_internal(traced_event, state, id) ); @@ -565,10 +557,7 @@ async function handle_remote_form_post_internal(event, state, id) { data.id = JSON.parse(decodeURIComponent(action_id)); } - await with_request_store( - { event, state: { ...state, is_in_remote_form_or_command: true } }, - () => __.fn(data, meta, form_data) - ); + await with_request_store({ event, state }, () => __.fn(data, meta, form_data)); // We don't want the data to appear on `let { form } = $props()`, which is why we're not returning it. // It is instead available on `myForm.result`, setting of which happens within the remote `form` function. diff --git a/packages/kit/src/runtime/server/remote-functions.spec.js b/packages/kit/src/runtime/server/remote-functions.spec.js index c09a4fe307b5..71340cceb8be 100644 --- a/packages/kit/src/runtime/server/remote-functions.spec.js +++ b/packages/kit/src/runtime/server/remote-functions.spec.js @@ -1,6 +1,6 @@ import { beforeAll, expect, test, vi } from 'vitest'; import { init_transport, parse } from '#app/internal/transport'; -import { get_request_store } from '@sveltejs/kit/internal/server'; +import { get_request_store, RequestEvent } from '@sveltejs/kit/internal/server'; const decoder = new TextDecoder(); @@ -24,9 +24,12 @@ beforeAll(async () => { * @param {(event: import('@sveltejs/kit').RequestEvent) => AsyncGenerator} run */ function create_response(run) { - const event = /** @type {import('@sveltejs/kit').RequestEvent} */ ({ - request: new Request('http://localhost/_app/remote/test?payload=undefined') - }); + const event = new RequestEvent( + /** @type {import('@sveltejs/kit').RequestEvent} */ ({ + request: new Request('http://localhost/_app/remote/test?payload=undefined') + }), + 0 + ); return create_live_query_response( event, @@ -116,13 +119,16 @@ test('serializes explicitly ignored requested updates', async () => { ); const response = await handle_remote_call( - /** @type {any} */ ({ - request: new Request('http://localhost/_app/remote/hash/command', { - method: 'POST', - body: JSON.stringify({ payload: '', refreshes: ['hash/query/[-1]'] }) + new RequestEvent( + /** @type {any} */ ({ + request: new Request('http://localhost/_app/remote/hash/command', { + method: 'POST', + body: JSON.stringify({ payload: '', refreshes: ['hash/query/[-1]'] }) + }), + tracing: { current: { setAttributes: vi.fn() } } }), - tracing: { current: { setAttributes: vi.fn() } } - }), + 0 + ), /** @type {any} */ ({ remote: { requested: null, ignored: null } }), 'hash/command' ); diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 48025d2c621c..0916765b2d15 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -2,12 +2,7 @@ import { DEV } from 'esm-env'; import { text } from '@sveltejs/kit'; import { Redirect, SvelteKitError } from '@sveltejs/kit/internal'; -import { - merge_tracing, - otel, - record_span, - with_request_store -} from '@sveltejs/kit/internal/server'; +import { otel, record_span, with_request_store, RequestEvent } from '@sveltejs/kit/internal/server'; import { base, app_dir } from '#app/paths'; import { is_endpoint_request, render_endpoint } from './endpoint.js'; import { render_page } from './page/index.js'; @@ -24,13 +19,13 @@ import { find_route } from '../../utils/routing.js'; import { redirect_json_response, render_data } from './data/index.js'; import { add_cookies_to_headers, get_cookies } from './cookie.js'; import { create_fetch } from './fetch.js'; +import { validateHeaders } from './validate-headers.js'; import { PageNodes } from '../../utils/page_nodes.js'; import { validate_server_exports } from '../../utils/exports.js'; import { action_json_redirect, is_action_json_request } from './page/actions.js'; import { INVALIDATED_PARAM, TRAILING_SLASH_PARAM } from '../shared.js'; import { get_public_env } from './env_module.js'; import { resolve_route, resolve_route_by_id } from './page/server_routing.js'; -import { validateHeaders } from './validate-headers.js'; import { add_data_suffix, add_resolution_suffix, @@ -182,70 +177,73 @@ export async function internal_respond(request, state) { /** @type {Record} */ const headers = {}; + let responded = false; const { cookies, new_cookies, get_cookie_header, set_internal, set_trailing_slash } = get_cookies( request, url ); - /** @type {import('@sveltejs/kit').RequestEvent} */ - const event = { - cookies, - // @ts-expect-error `fetch` needs to be created after the `event` itself - fetch: null, - getClientAddress: - state.getClientAddress || - (() => { - throw new Error( - `${__SVELTEKIT_ADAPTER_NAME__} does not specify getClientAddress. Please raise an issue` - ); - }), - locals: {}, - params: {}, - platform: state.emulator?.platform - ? await state.emulator.platform({ - config: {}, - prerender: !!state.prerendering?.fallback - }) - : state.platform, - request, - route: { id: null }, - setHeaders: (new_headers) => { - if (DEV) { - validateHeaders(new_headers); - } - - for (const key in new_headers) { - const lower = key.toLowerCase(); - const value = new_headers[key]; - - if (lower === 'set-cookie') { + const event = new RequestEvent( + /** @type {import('@sveltejs/kit').RequestEvent} */ ({ + cookies, + getClientAddress: + state.getClientAddress || + (() => { throw new Error( - 'Use `event.cookies.set(name, value, options)` instead of `event.setHeaders` to set cookies' + `${__SVELTEKIT_ADAPTER_NAME__} does not specify getClientAddress. Please raise an issue` ); - } else if (lower in headers) { - // appendHeaders-style for Server-Timing https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Server-Timing - if (lower === 'server-timing') { - headers[lower] += ', ' + value; + }), + locals: {}, + params: {}, + platform: state.emulator?.platform + ? await state.emulator.platform({ + config: {}, + prerender: !!state.prerendering?.fallback + }) + : state.platform, + request, + route: { id: null }, + setHeaders: (new_headers) => { + if (responded) { + throw new Error('Cannot use `setHeaders(...)` after the response has been generated'); + } + + if (DEV) { + validateHeaders(new_headers); + } + + for (const [key, value] of Object.entries(new_headers)) { + const lower = key.toLowerCase(); + + if (lower === 'set-cookie') { + throw new Error( + 'Use `event.cookies.set(name, value, options)` instead of `event.setHeaders` to set cookies' + ); + } else if (Object.hasOwn(headers, lower)) { + // appendHeaders-style for Server-Timing https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Server-Timing + if (lower === 'server-timing') { + headers[lower] += ', ' + value; + } else { + throw new Error(`"${key}" header is already set`); + } } else { - throw new Error(`"${key}" header is already set`); - } - } else { - headers[lower] = value; + headers[lower] = value; - if (state.prerendering && lower === 'cache-control') { - state.prerendering.cache = /** @type {string} */ (value); + if (state.prerendering && lower === 'cache-control') { + state.prerendering.cache = value; + } } } - } - }, - url, - isDataRequest: is_data_request, - isSubRequest: state.depth > 0, - isRemoteRequest: !!remote_id - }; + }, + url, + isDataRequest: is_data_request, + isSubRequest: state.depth > 0, + isRemoteRequest: !!remote_id + }), + 0 + ); - // @ts-expect-error this has to be assigned lazily event.fetch = create_fetch({ event, state, @@ -375,9 +373,7 @@ export async function internal_respond(request, state) { if (result) { route = result.route; - // @ts-expect-error this has to be assigned lazily event.route = { id: route.id }; - // @ts-expect-error this has to be assigned lazily event.params = result.params; } } catch (e) { @@ -439,7 +435,6 @@ export async function internal_respond(request, state) { } if (state.emulator?.platform) { - // @ts-expect-error this has to be assigned lazily event.platform = await state.emulator.platform({ config, prerender }); } @@ -485,13 +480,11 @@ export async function internal_respond(request, state) { 'sveltekit.is_sub_request': event.isSubRequest }, fn: async (root_span) => { - const traced_event = { - ...event, - tracing: { - enabled: __SVELTEKIT_SERVER_TRACING_ENABLED__, - root: root_span, - current: root_span - } + const traced_event = event.clone(); + traced_event.tracing = { + enabled: __SVELTEKIT_SERVER_TRACING_ENABLED__, + root: root_span, + current: root_span }; return await with_request_store({ event: traced_event, state }, () => @@ -504,33 +497,32 @@ export async function internal_respond(request, state) { 'http.route': event.route.id || 'unknown' }, fn: (resolve_span) => { + const traced_event = RequestEvent.from(event, resolve_span); + // counter-intuitively, we need to clear the event, so that it's not // e.g. accessible when loading modules needed to handle the request return with_request_store(null, () => - resolve(merge_tracing(event, resolve_span), page_nodes, opts).then( - (response) => { - // add headers/cookies here, rather than inside `resolve`, so that we - // can do it once for all responses instead of once per `return` - for (const key in headers) { - const value = headers[key]; - response.headers.set(key, /** @type {string} */ (value)); - } - - add_cookies_to_headers(response.headers, new_cookies.values()); - - if (state.prerendering && event.route.id !== null) { - response.headers.set('x-sveltekit-routeid', encodeURI(event.route.id)); - } - - resolve_span.setAttributes({ - 'http.response.status_code': response.status, - 'http.response.body.size': - response.headers.get('content-length') || 'unknown' - }); - - return response; + resolve(traced_event, page_nodes, opts).then((response) => { + // add headers/cookies here, rather than inside `resolve`, so that we + // can do it once for all responses instead of once per `return` + for (const [key, value] of Object.entries(headers)) { + response.headers.set(key, value); + } + + add_cookies_to_headers(response.headers, new_cookies.values()); + + if (state.prerendering && event.route.id !== null) { + response.headers.set('x-sveltekit-routeid', encodeURI(event.route.id)); } - ) + + resolve_span.setAttributes({ + 'http.response.status_code': response.status, + 'http.response.body.size': + response.headers.get('content-length') || 'unknown' + }); + + return response; + }) ); } }); @@ -584,7 +576,7 @@ export async function internal_respond(request, state) { } /** - * @param {import('@sveltejs/kit').RequestEvent} event + * @param {import('@sveltejs/kit/internal/server').RequestEvent} event * @param {PageNodes | undefined} page_nodes * @param {import('@sveltejs/kit/hooks').ResolveOptions} [opts] */ @@ -794,14 +786,10 @@ export async function internal_respond(request, state) { // HttpError from endpoint can end up here - TODO should it be handled there instead? return await handle_fatal_error(event, state, e); } finally { + responded = true; event.cookies.set = () => { throw new Error('Cannot use `cookies.set(...)` after the response has been generated'); }; - - // @ts-expect-error this has to be assigned lazily - event.setHeaders = () => { - throw new Error('Cannot use `setHeaders(...)` after the response has been generated'); - }; } } } diff --git a/packages/kit/src/runtime/server/state.js b/packages/kit/src/runtime/server/state.js index 77905dc4ac88..18a69516346d 100644 --- a/packages/kit/src/runtime/server/state.js +++ b/packages/kit/src/runtime/server/state.js @@ -1,6 +1,6 @@ /** @import { InternalRequestOptions, RequestState } from 'types' */ -/** Per-request caches and context flags — never carried into a fork. */ +/** Per-request caches — never carried into a fork. */ function transient_fields() { return { remote: { @@ -12,12 +12,7 @@ function transient_fields() { ignored: null, batches: null, live_iterators: null - }, - is_in_remote_function: false, - is_in_remote_form_or_command: false, - is_in_remote_query: false, - is_in_remote_prerender: false, - is_in_render: false + } }; } diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index 1d49f5c97775..977a52792032 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -783,15 +783,10 @@ export interface RequestState { */ live_iterators: null | Map>; }; - readonly is_in_remote_function: boolean; - readonly is_in_remote_form_or_command: boolean; - readonly is_in_remote_query: boolean; - readonly is_in_remote_prerender: boolean; - readonly is_in_render: boolean; } export interface RequestStore { - event: RequestEvent; + event: import('../exports/internal/server/event.js').RequestEvent; state: RequestState; } diff --git a/packages/kit/test/apps/async/src/routes/remote/server-endpoint/internal.remote.ts b/packages/kit/test/apps/async/src/routes/remote/server-endpoint/internal.remote.ts index 25e83a63bee9..fac747945208 100644 --- a/packages/kit/test/apps/async/src/routes/remote/server-endpoint/internal.remote.ts +++ b/packages/kit/test/apps/async/src/routes/remote/server-endpoint/internal.remote.ts @@ -1,9 +1,10 @@ -import { command, query } from '$app/server'; +import { command, query, requested } from '$app/server'; export const get = query(() => { return 'get'; }); -export const add = command('unchecked', () => { +export const add = command('unchecked', async () => { + await requested(get, 1).refreshAll(); return 'post'; }); diff --git a/packages/kit/test/apps/basics/src/hooks.server.js b/packages/kit/test/apps/basics/src/hooks.server.js index 2f35ad2d1e0b..8e4244249dc2 100644 --- a/packages/kit/test/apps/basics/src/hooks.server.js +++ b/packages/kit/test/apps/basics/src/hooks.server.js @@ -89,6 +89,17 @@ export const handle = sequence( event.locals.answer = 42; return resolve(event); }, + async ({ event, resolve }) => { + const response = await resolve(event); + if (event.request.url.includes('?set-headers-after-resolve')) { + try { + event.setHeaders({ 'x-late': '1' }); + } catch (e) { + return new Response(/** @type {Error} */ (e).message); + } + } + return response; + }, ({ event, resolve }) => { if ( event.request.url.includes('__data.json') && diff --git a/packages/kit/test/apps/basics/test/playwright/test.js b/packages/kit/test/apps/basics/test/playwright/test.js index 86e81e0426fc..dd4a46270c43 100644 --- a/packages/kit/test/apps/basics/test/playwright/test.js +++ b/packages/kit/test/apps/basics/test/playwright/test.js @@ -1738,6 +1738,15 @@ test.describe('getRequestEvent', () => { }); }); +test.describe('setHeaders', () => { + test('throws once the response has been generated', async ({ request }) => { + const response = await request.get('/?set-headers-after-resolve'); + expect(await response.text()).toBe( + 'Cannot use `setHeaders(...)` after the response has been generated' + ); + }); +}); + test.describe('params prop', () => { test('params prop is passed to the page', async ({ page, clicknav }) => { await page.goto('/params-prop');