Improve errors for invalid IDs in content collections (#12892)

* Improve error handling for content collection entries where ID isn't a string

* Add passthrough to zod schema to still access other properties after validation

* Add new test for numbers as IDs, add changeset

* Update error for invalid IDs

* Update test for new error message

* Update .changeset/dry-dragons-shout.md

Co-authored-by: Sarah Rainsberger <[email protected]>

* Update errors-data.ts to (possibly) fix tests failing

* Update errors-data.ts

---------

Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Matt Kane <[email protected]>

.changeset/dry-dragons-shout.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Adds a more descriptive error when a content collection entry has an invalid ID.

packages/astro/src/content/content-layer.ts

@@ -20,8 +20,10 @@ import {
 	getEntryConfigByExtMap,
 	getEntryDataAndImages,
 	globalContentConfigObserver,
+	loaderReturnSchema,
 	safeStringify,
 } from './utils.js';
+import type { z } from 'zod';
 import { type WrappedWatcher, createWatcherWrapper } from './watcher.js';
 
 export interface ContentLayerOptions {
@@ -31,6 +33,12 @@ export interface ContentLayerOptions {
 	watcher?: FSWatcher;
 }
 
+type CollectionLoader<TData> = () =>
+	| Array<TData>
+	| Promise<Array<TData>>
+	| Record<string, Record<string, unknown>>
+	| Promise<Record<string, Record<string, unknown>>>;
+
 export class ContentLayer {
 	#logger: Logger;
 	#store: MutableDataStore;
@@ -276,7 +284,7 @@ export class ContentLayer {
 				});
 
 				if (typeof collection.loader === 'function') {
-					return simpleLoader(collection.loader, context);
+					return simpleLoader(collection.loader as CollectionLoader<{ id: string }>, context);
 				}
 
 				if (!collection.loader.load) {
@@ -334,15 +342,38 @@ export class ContentLayer {
 }
 
 export async function simpleLoader<TData extends { id: string }>(
-	handler: () =>
-		| Array<TData>
-		| Promise<Array<TData>>
-		| Record<string, Record<string, unknown>>
-		| Promise<Record<string, Record<string, unknown>>>,
+	handler: CollectionLoader<TData>,
 	context: LoaderContext,
 ) {
-	const data = await handler();
+	const unsafeData = await handler();
+	const parsedData = loaderReturnSchema.safeParse(unsafeData);
+
+	if (!parsedData.success) {
+		const issue = parsedData.error.issues[0] as z.ZodInvalidUnionIssue;
+
+		// Due to this being a union, zod will always throw an "Expected array, received object" error along with the other errors.
+		// This error is in the second position if the data is an array, and in the first position if the data is an object.
+		const parseIssue = Array.isArray(unsafeData) 
+			? issue.unionErrors[0] 
+			: issue.unionErrors[1];
+
+		const error = parseIssue.errors[0];
+		const firstPathItem = error.path[0];
+
+		const entry = Array.isArray(unsafeData) 
+			? unsafeData[firstPathItem as number] 
+			: unsafeData[firstPathItem as string];
+
+		throw new AstroError({
+			...AstroErrorData.ContentLoaderReturnsInvalidId,
+			message: AstroErrorData.ContentLoaderReturnsInvalidId.message(context.collection, entry),
+		});
+	}
+
+	const data = parsedData.data;
+
 	context.store.clear();
+
 	if (Array.isArray(data)) {
 		for (const raw of data) {
 			if (!raw.id) {

packages/astro/src/content/utils.ts

@@ -26,8 +26,9 @@ import {
 } from './consts.js';
 import { glob } from './loaders/glob.js';
 import { createImage } from './runtime-assets.js';
+
 /**
- * Amap from a collection + slug to the local file path.
+ * A map from a collection + slug to the local file path.
  * This is used internally to resolve entry imports when using `getEntry()`.
  * @see `templates/content/module.mjs`
  */
@@ -41,10 +42,20 @@ const entryTypeSchema = z
 			.string({
 				invalid_type_error: 'Content entry `id` must be a string',
 				// Default to empty string so we can validate properly in the loader
-			})
-			.catch(''),
-	})
-	.catchall(z.unknown());
+			}),
+	}).passthrough();
+
+export const loaderReturnSchema = z.union([
+	z.array(entryTypeSchema),
+	z.record(
+		z.string(), 
+		z.object({ 
+			id: z.string({ 
+				invalid_type_error: 'Content entry `id` must be a string' 
+			}).optional() 
+		}).passthrough()
+	),
+]);
 
 const collectionConfigParser = z.union([
 	z.object({
@@ -59,39 +70,7 @@ const collectionConfigParser = z.union([
 		type: z.literal(CONTENT_LAYER_TYPE),
 		schema: z.any().optional(),
 		loader: z.union([
-			z.function().returns(
-				z.union([
-					z.array(entryTypeSchema),
-					z.promise(z.array(entryTypeSchema)),
-					z.record(
-						z.string(),
-						z
-							.object({
-								id: z
-									.string({
-										invalid_type_error: 'Content entry `id` must be a string',
-									})
-									.optional(),
-							})
-							.catchall(z.unknown()),
-					),
-
-					z.promise(
-						z.record(
-							z.string(),
-							z
-								.object({
-									id: z
-										.string({
-											invalid_type_error: 'Content entry `id` must be a string',
-										})
-										.optional(),
-								})
-								.catchall(z.unknown()),
-						),
-					),
-				]),
-			),
+			z.function(),
 			z.object({
 				name: z.string(),
 				load: z.function(

packages/astro/src/core/errors/errors-data.ts

@@ -1565,6 +1565,32 @@ export const InvalidContentEntryDataError = {
 	hint: 'See https://docs.astro.build/en/guides/content-collections/ for more information on content schemas.',
 } satisfies ErrorData;
 
+/**
+ * @docs
+ * @message
+ * **Example error message:**<br/>
+ * The content loader for the collection **blog** returned an entry with an invalid `id`:<br/>
+ * &#123;<br/>
+ *   "id": 1,<br/>
+ *   "title": "Hello, World!"<br/>
+ * &#125;
+ * @description
+ * A content loader returned an invalid `id`.
+ * Make sure that the `id` of the entry is a string.
+ * See the [Content collections documentation](https://docs.astro.build/en/guides/content-collections/) for more information.
+ */
+export const ContentLoaderReturnsInvalidId = {
+	name: 'ContentLoaderReturnsInvalidId',
+	title: 'Content loader returned an entry with an invalid `id`.',
+	message(collection: string, entry: any) {
+		return [
+			`The content loader for the collection **${String(collection)}** returned an entry with an invalid \`id\`:`,
+			JSON.stringify(entry, null, 2),
+		].join('\n');
+	},
+	hint: 'Make sure that the `id` of the entry is a string. See https://docs.astro.build/en/guides/content-collections/ for more information on content loaders.',
+} satisfies ErrorData;
+
 /**
  * @docs
  * @message

packages/astro/test/content-collections.test.js

@@ -300,6 +300,21 @@ describe('Content Collections', () => {
 		});
 	});
 
+	describe('With numbers for IDs', () => {
+		it('Throws the right error', async () => {
+			const fixture = await loadFixture({
+				root: './fixtures/content-collections-number-id/',
+			});
+			let error;
+			try {
+				await fixture.build({ force: true });
+			} catch (e) {
+				error = e.message;
+			}
+			assert.match(error, /returned an entry with an invalid `id`/); 
+		});
+	});
+
 	describe('With empty collections directory', () => {
 		it('Handles the empty directory correctly', async () => {
 			const fixture = await loadFixture({

packages/astro/test/fixtures/content-collections-number-id/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@test/content-collections-number-id",
+  "version": "0.0.0",
+  "private": true,
+  "type": "module",
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/content-collections-number-id/src/content.config.ts

@@ -0,0 +1,17 @@
+import { defineCollection, z } from 'astro:content';
+
+const data = defineCollection({
+	loader: async () => ([
+		{ id: 1, title: 'One!' },
+		{ id: 2, title: 'Two!' },
+		{ id: 3, title: 'Three!' },
+	]),
+	schema: z.object({
+		id: z.number(),
+		title: z.string(),
+	}),
+});
+
+export const collections = {
+	data,
+};

packages/astro/test/fixtures/content-collections-number-id/src/pages/index.astro

@@ -0,0 +1,6 @@
+---
+import { getEntry } from 'astro:content';
+const data = await getEntry('blog', 1);
+---
+
+{JSON.stringify(data)}