fix: try to get session id from cookie when destroying an unloaded session (#14167)

Author: ascorbic

.changeset/soft-rabbits-yell.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug that prevented destroyed sessions from being deleted from storage unless the session had been loaded

packages/astro/src/core/session.ts

@@ -214,9 +214,16 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
 	/**
 	 * Destroys the session, clearing the cookie and storage if it exists.
 	 */
-
 	destroy() {
-		this.#destroySafe();
+		// We don't use #ensureSessionID here because we don't want to create a new session ID if it doesn't exist.
+		const sessionId = this.#sessionID ?? this.#cookies.get(this.#cookieName)?.value;
+		if (sessionId) {
+			this.#toDestroy.add(sessionId);
+		}
+		this.#cookies.delete(this.#cookieName, this.#cookieConfig);
+		this.#sessionID = undefined;
+		this.#data = undefined;
+		this.#dirty = true;
 	}
 
 	/**
@@ -351,7 +358,7 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
 		try {
 			const storedMap = unflatten(raw);
 			if (!(storedMap instanceof Map)) {
-				await this.#destroySafe();
+				await this.destroy();
 				throw new AstroError({
 					...SessionStorageInitError,
 					message: SessionStorageInitError.message(
@@ -377,7 +384,7 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
 			this.#partial = false;
 			return this.#data;
 		} catch (err) {
-			await this.#destroySafe();
+			await this.destroy();
 			if (err instanceof AstroError) {
 				throw err;
 			}
@@ -393,20 +400,6 @@ export class AstroSession<TDriver extends SessionDriverName = any> {
 			);
 		}
 	}
-	/**
-	 * Safely destroys the session, clearing the cookie and storage if it exists.
-	 */
-	#destroySafe() {
-		if (this.#sessionID) {
-			this.#toDestroy.add(this.#sessionID);
-		}
-		if (this.#cookieName) {
-			this.#cookies.delete(this.#cookieName, this.#cookieConfig);
-		}
-		this.#sessionID = undefined;
-		this.#data = undefined;
-		this.#dirty = true;
-	}
 
 	/**
 	 * Returns the session ID, generating a new one if it does not exist.

packages/astro/test/units/sessions/astro-session.test.js

@@ -7,7 +7,7 @@ import { AstroSession, PERSIST_SYMBOL } from '../../../dist/core/session.js';
 const defaultMockCookies = {
 	set: () => {},
 	delete: () => {},
-	get: () => 'sessionid',
+	get: () => ({ value: 'sessionid' }),
 };
 
 const stringify = (data) => JSON.parse(devalueStringify(data));
@@ -360,6 +360,24 @@ test('AstroSession - Cleanup Operations', async (t) => {
 
 		assert.ok(removedKeys.has(oldId), `Session ${oldId} should be removed`);
 	});
+
+	await t.test("should destroy sessions that haven't been loaded", async () => {
+		const removedKeys = new Set();
+		const mockStorage = {
+			get: async () => stringify(new Map([['key', 'value']])),
+			setItem: async () => {},
+			removeItem: async (key) => {
+				removedKeys.add(key);
+			},
+		};
+
+		const session = createSession(defaultConfig, defaultMockCookies, mockStorage);
+		session.destroy();
+
+		// Simulate end of request
+		await session[PERSIST_SYMBOL]();
+		assert.equal(removedKeys.size, 1, `Session should be removed`);
+	});
 });
 
 test('AstroSession - Cookie Security', async (t) => {