chore: keep the call context on the event view instead of cloning the request state

.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

.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

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<Response>}
 		 */
@@ -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,

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} */ ({}))
 		})
 	};

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<Interface, 'url' | 'params' | 'route'>}
+ */
+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();

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'
+	);
+});