Actions: fix custom error message on client (#11030)

* feat(test): error throwing on server * feat: correctly parse custom errors for the client * feat(test): custom errors on client * chore: changeset
withastro · May 13, 2024 · 18e7f33 · 18e7f33
1 parent c135cd5
commit 18e7f33
Show file tree

Hide file tree

Showing 11 changed files with 112 additions and 31 deletions.
diff --git a/.changeset/slimy-comics-thank.md b/.changeset/slimy-comics-thank.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Actions: Fix missing message for custom Action errors.
diff --git a/packages/astro/e2e/actions-blog.test.js b/packages/astro/e2e/actions-blog.test.js
@@ -38,6 +38,22 @@ test.describe('Astro Actions - Blog', () => {
 		await expect(page.locator('p[data-error="body"]')).toBeVisible();
 	});
 
+	test('Comment action - custom error', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/blog/first-post/?commentPostIdOverride=bogus'));
+
+		const authorInput = page.locator('input[name="author"]');
+		const bodyInput = page.locator('textarea[name="body"]');
+		await authorInput.fill('Ben');
+		await bodyInput.fill('This should be long enough.');
+
+		const submitButton = page.getByLabel('Post comment');
+		await submitButton.click();
+
+		const unexpectedError = page.locator('p[data-error="unexpected"]');
+		await expect(unexpectedError).toBeVisible();
+		await expect(unexpectedError).toContainText('NOT_FOUND: Post not found');
+	});
+
 	test('Comment action - success', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/blog/first-post/'));
 

diff --git a/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts b/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts
@@ -1,5 +1,6 @@
 import { db, Comment, Likes, eq, sql } from 'astro:db';
-import { defineAction, z } from 'astro:actions';
+import { ActionError, defineAction, z } from 'astro:actions';
+import { getCollection } from 'astro:content';
 
 export const server = {
 	blog: {
@@ -29,6 +30,13 @@ export const server = {
 				body: z.string().min(10),
 			}),
 			handler: async ({ postId, author, body }) => {
+				if (!(await getCollection('blog')).find(b => b.id === postId)) {
+					throw new ActionError({
+						code: 'NOT_FOUND',
+						message: 'Post not found',
+					});
+				}
+
 				const comment = await db
 					.insert(Comment)
 					.values({

diff --git a/packages/astro/e2e/fixtures/actions-blog/src/components/PostComment.tsx b/packages/astro/e2e/fixtures/actions-blog/src/components/PostComment.tsx
@@ -10,6 +10,7 @@ export function PostComment({
 }) {
 	const [comments, setComments] = useState<{ author: string; body: string }[]>([]);
 	const [bodyError, setBodyError] = useState<string | undefined>(serverBodyError);
+	const [unexpectedError, setUnexpectedError] = useState<string | undefined>(undefined);
 
 	return (
 		<>
@@ -22,14 +23,15 @@ export function PostComment({
 					const { data, error } = await actions.blog.comment.safe(formData);
 					if (isInputError(error)) {
 						return setBodyError(error.fields.body?.join(' '));
+					} else if (error) {
+						return setUnexpectedError(`${error.code}: ${error.message}`);
 					}
-					if (data) {
-						setBodyError(undefined);
-						setComments((c) => [data, ...c]);
-					}
+					setBodyError(undefined);
+					setComments((c) => [data, ...c]);
 					form.reset();
 				}}
 			>
+				{unexpectedError && <p data-error="unexpected" style={{ color: 'red' }}>{unexpectedError}</p>}
 				<input {...getActionProps(actions.blog.comment)} />
 				<input type="hidden" name="postId" value={postId} />
 				<label className="sr-only" htmlFor="author">

diff --git a/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro b/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
@@ -27,6 +27,9 @@ const comment = Astro.getActionResult(actions.blog.comment);
 const comments = await db.select().from(Comment).where(eq(Comment.postId, post.id));
 
 const initialLikes = await db.select().from(Likes).where(eq(Likes.postId, post.id)).get();
+
+// Used to force validation errors for testing
+const commentPostIdOverride = Astro.url.searchParams.get('commentPostIdOverride');
 ---
 
 <BlogPost {...post.data}>
@@ -36,7 +39,7 @@ const initialLikes = await db.select().from(Likes).where(eq(Likes.postId, post.i
 
 	<h2>Comments</h2>
 	<PostComment
-		postId={post.id}
+		postId={commentPostIdOverride ?? post.id}
 		serverBodyError={isInputError(comment?.error)
 			? comment.error.fields.body?.toString()
 			: undefined}

diff --git a/packages/astro/src/actions/runtime/route.ts b/packages/astro/src/actions/runtime/route.ts
@@ -20,16 +20,19 @@ export const POST: APIRoute = async (context) => {
 	}
 	const result = await ApiContextStorage.run(context, () => callSafely(() => action(args)));
 	if (result.error) {
-		if (import.meta.env.PROD) {
-			// Avoid leaking stack trace in production
-			result.error.stack = undefined;
-		}
-		return new Response(JSON.stringify(result.error), {
-			status: result.error.status,
-			headers: {
-				'Content-Type': 'application/json',
-			},
-		});
+		return new Response(
+			JSON.stringify({
+				...result.error,
+				message: result.error.message,
+				stack: import.meta.env.PROD ? undefined : result.error.stack,
+			}),
+			{
+				status: result.error.status,
+				headers: {
+					'Content-Type': 'application/json',
+				},
+			}
+		);
 	}
 	return new Response(JSON.stringify(result.data), {
 		headers: {

diff --git a/packages/astro/src/actions/runtime/virtual/shared.ts b/packages/astro/src/actions/runtime/virtual/shared.ts
@@ -47,10 +47,13 @@ export class ActionError<T extends ErrorInferenceObject = ErrorInferenceObject>
 	code: ActionErrorCode = 'INTERNAL_SERVER_ERROR';
 	status = 500;
 
-	constructor(params: { message?: string; code: ActionErrorCode }) {
+	constructor(params: { message?: string; code: ActionErrorCode; stack?: string }) {
 		super(params.message);
 		this.code = params.code;
 		this.status = ActionError.codeToStatus(params.code);
+		if (params.stack) {
+			this.stack = params.stack;
+		}
 	}
 
 	static codeToStatus(code: ActionErrorCode): number {
@@ -62,22 +65,20 @@ export class ActionError<T extends ErrorInferenceObject = ErrorInferenceObject>
 	}
 
 	static async fromResponse(res: Response) {
+		const body = await res.clone().json();
 		if (
-			res.status === 400 &&
-			res.headers.get('Content-Type')?.toLowerCase().startsWith('application/json')
+			typeof body === 'object' &&
+			body?.type === 'AstroActionInputError' &&
+			Array.isArray(body.issues)
 		) {
-			const body = await res.json();
-			if (
-				typeof body === 'object' &&
-				body?.type === 'AstroActionInputError' &&
-				Array.isArray(body.issues)
-			) {
-				return new ActionInputError(body.issues);
-			}
+			return new ActionInputError(body.issues);
+		}
+		if (typeof body === 'object' && body?.type === 'AstroActionError') {
+			return new ActionError(body);
 		}
 		return new ActionError({
 			message: res.statusText,
-			code: this.statusToCode(res.status),
+			code: ActionError.statusToCode(res.status),
 		});
 	}
 }

diff --git a/packages/astro/test/actions.test.js b/packages/astro/test/actions.test.js
@@ -174,7 +174,7 @@ describe('Astro Actions', () => {
 		it('Respects user middleware', async () => {
 			const formData = new FormData();
 			formData.append('_astroAction', '/_actions/getUser');
-			const req = new Request('http://example.com/middleware', {
+			const req = new Request('http://example.com/user', {
 				method: 'POST',
 				body: formData,
 			});
@@ -185,5 +185,22 @@ describe('Astro Actions', () => {
 			let $ = cheerio.load(html);
 			assert.equal($('#user').text(), 'Houston');
 		});
+
+		it('Respects custom errors', async () => {
+			const formData = new FormData();
+			formData.append('_astroAction', '/_actions/getUserOrThrow');
+			const req = new Request('http://example.com/user-or-throw', {
+				method: 'POST',
+				body: formData,
+			});
+			const res = await app.render(req);
+			assert.equal(res.ok, false);
+			assert.equal(res.status, 401);
+
+			const html = await res.text();
+			let $ = cheerio.load(html);
+			assert.equal($('#error-message').text(), 'Not logged in');
+			assert.equal($('#error-code').text(), 'UNAUTHORIZED');
+		})
 	});
 });
diff --git a/packages/astro/test/fixtures/actions/src/actions/index.ts b/packages/astro/test/fixtures/actions/src/actions/index.ts
@@ -1,4 +1,4 @@
-import { defineAction, getApiContext, z } from 'astro:actions';
+import { defineAction, getApiContext, ActionError, z } from 'astro:actions';
 
 export const server = {
 	subscribe: defineAction({
@@ -35,5 +35,19 @@ export const server = {
 			const { locals } = getApiContext();
 			return locals.user;
 		}
-	})
+	}),
+	getUserOrThrow: defineAction({
+		accept: 'form',
+		handler: async () => {
+			const { locals } = getApiContext();
+			if (locals.user?.name !== 'admin') {
+				// Expected to throw
+				throw new ActionError({
+					code: 'UNAUTHORIZED',
+					message: 'Not logged in',
+				});
+			}
+			return locals.user;
+		}
+	}),
 };
diff --git a/packages/astro/test/fixtures/actions/src/pages/user-or-throw.astro b/packages/astro/test/fixtures/actions/src/pages/user-or-throw.astro
@@ -0,0 +1,12 @@
+---
+import { actions } from 'astro:actions';
+
+const res = Astro.getActionResult(actions.getUserOrThrow);
+
+if (res?.error) {
+	Astro.response.status = res.error.status;
+}
+---
+
+<p id="error-message">{res?.error?.message}</p>
+<p id="error-code">{res?.error?.code}</p>
diff --git a/...xtures/actions/src/pages/middleware.astro → ...est/fixtures/actions/src/pages/user.astro b/...xtures/actions/src/pages/middleware.astro → ...est/fixtures/actions/src/pages/user.astro