summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--packages/astro/CHANGELOG.md25
-rw-r--r--packages/astro/src/actions/runtime/utils.ts7
-rw-r--r--packages/astro/src/core/constants.ts5
-rw-r--r--packages/astro/src/core/errors/dev/vite.ts2
-rw-r--r--packages/astro/src/core/middleware/noop-middleware.ts6
-rw-r--r--packages/astro/src/core/module-loader/vite.ts21
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts46
-rw-r--r--packages/astro/test/actions.test.js6
-rw-r--r--packages/astro/test/fixtures/actions/src/pages/invalid.astro6
-rw-r--r--packages/astro/test/fixtures/middleware space/src/middleware.js11
-rw-r--r--packages/astro/test/middleware.test.js15
11 files changed, 129 insertions, 21 deletions
diff --git a/packages/astro/CHANGELOG.md b/packages/astro/CHANGELOG.md
index 98332e7da..aa3d02e32 100644
--- a/packages/astro/CHANGELOG.md
+++ b/packages/astro/CHANGELOG.md
@@ -1126,6 +1126,31 @@
- Updated dependencies [[`83a2a64`](https://github.com/withastro/astro/commit/83a2a648418ad30f4eb781d1c1b5f2d8a8ac846e)]:
- @astrojs/markdown-remark@6.0.0-alpha.0
+## 4.16.12
+
+### Patch Changes
+
+- [#12420](https://github.com/withastro/astro/pull/12420) [`acac0af`](https://github.com/withastro/astro/commit/acac0af53466f8a381ccdac29ed2ad735d7b4e79) Thanks [@ematipico](https://github.com/ematipico)! - Fixes an issue where the dev server returns a 404 status code when a user middleware returns a valid `Response`.
+
+## 4.16.11
+
+### Patch Changes
+
+- [#12305](https://github.com/withastro/astro/pull/12305) [`f5f7109`](https://github.com/withastro/astro/commit/f5f71094ec74961b4cca2ee451798abd830c617a) Thanks [@florian-lefebvre](https://github.com/florian-lefebvre)! - Fixes a case where the error overlay would not escape the message
+
+- [#12402](https://github.com/withastro/astro/pull/12402) [`823e73b`](https://github.com/withastro/astro/commit/823e73b164eab4115af31b1de8e978f2b4e0a95d) Thanks [@ematipico](https://github.com/ematipico)! - Fixes a case where Astro allowed to call an action without using `Astro.callAction`. This is now invalid, and Astro will show a proper error.
+
+ ```diff
+ ---
+ import { actions } from "astro:actions";
+
+ -const result = actions.getUser({ userId: 123 });
+ +const result = Astro.callAction(actions.getUser, { userId: 123 });
+ ---
+ ```
+
+- [#12401](https://github.com/withastro/astro/pull/12401) [`9cca108`](https://github.com/withastro/astro/commit/9cca10843912698e13d35f1bc3c493e2c96a06ee) Thanks [@bholmesdev](https://github.com/bholmesdev)! - Fixes unexpected 200 status in dev server logs for action errors and redirects.
+
## 4.16.10
### Patch Changes
diff --git a/packages/astro/src/actions/runtime/utils.ts b/packages/astro/src/actions/runtime/utils.ts
index 7d6a21730..35c750055 100644
--- a/packages/astro/src/actions/runtime/utils.ts
+++ b/packages/astro/src/actions/runtime/utils.ts
@@ -10,6 +10,8 @@ export type Locals = {
_actionPayload: ActionPayload;
};
+export const ACTION_API_CONTEXT_SYMBOL = Symbol.for('astro.actionAPIContext');
+
export const formContentTypes = ['application/x-www-form-urlencoded', 'multipart/form-data'];
export function hasContentType(contentType: string, expected: string[]) {
@@ -36,3 +38,8 @@ export type MaybePromise<T> = T | Promise<T>;
* `result.error.fields` will be typed with the `name` field.
*/
export type ErrorInferenceObject = Record<string, any>;
+
+export function isActionAPIContext(ctx: ActionAPIContext): boolean {
+ const symbol = Reflect.get(ctx, ACTION_API_CONTEXT_SYMBOL);
+ return symbol === true;
+}
diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts
index 6183e1557..dc09a1f69 100644
--- a/packages/astro/src/core/constants.ts
+++ b/packages/astro/src/core/constants.ts
@@ -32,6 +32,11 @@ export const REWRITE_DIRECTIVE_HEADER_KEY = 'X-Astro-Rewrite';
export const REWRITE_DIRECTIVE_HEADER_VALUE = 'yes';
/**
+ * This header is set by the no-op Astro middleware.
+ */
+export const NOOP_MIDDLEWARE_HEADER = 'X-Astro-Noop';
+
+/**
* The name for the header used to help i18n middleware, which only needs to act on "page" and "fallback" route types.
*/
export const ROUTE_TYPE_HEADER = 'X-Astro-Route-Type';
diff --git a/packages/astro/src/core/errors/dev/vite.ts b/packages/astro/src/core/errors/dev/vite.ts
index c944ad78d..44b41e379 100644
--- a/packages/astro/src/core/errors/dev/vite.ts
+++ b/packages/astro/src/core/errors/dev/vite.ts
@@ -105,6 +105,7 @@ export function enhanceViteSSRError({
}
export interface AstroErrorPayload {
+ __isEnhancedAstroErrorPayload: true;
type: ErrorPayload['type'];
err: Omit<ErrorPayload['err'], 'loc'> & {
name?: string;
@@ -164,6 +165,7 @@ export async function getViteErrorPayload(err: ErrorWithMetadata): Promise<Astro
: undefined;
return {
+ __isEnhancedAstroErrorPayload: true,
type: 'error',
err: {
...err,
diff --git a/packages/astro/src/core/middleware/noop-middleware.ts b/packages/astro/src/core/middleware/noop-middleware.ts
index ebb7c1b34..d8ccbc2c7 100644
--- a/packages/astro/src/core/middleware/noop-middleware.ts
+++ b/packages/astro/src/core/middleware/noop-middleware.ts
@@ -1,3 +1,7 @@
import type { MiddlewareHandler } from '../../types/public/common.js';
+import { NOOP_MIDDLEWARE_HEADER } from '../constants.js';
-export const NOOP_MIDDLEWARE_FN: MiddlewareHandler = (_, next) => next();
+export const NOOP_MIDDLEWARE_FN: MiddlewareHandler = (ctx, next) => {
+ ctx.request.headers.set(NOOP_MIDDLEWARE_HEADER, 'true');
+ return next();
+};
diff --git a/packages/astro/src/core/module-loader/vite.ts b/packages/astro/src/core/module-loader/vite.ts
index 1b2a4423c..20aea87e2 100644
--- a/packages/astro/src/core/module-loader/vite.ts
+++ b/packages/astro/src/core/module-loader/vite.ts
@@ -1,6 +1,9 @@
import { EventEmitter } from 'node:events';
import path from 'node:path';
+import { pathToFileURL } from 'node:url';
import type * as vite from 'vite';
+import { collectErrorMetadata } from '../errors/dev/utils.js';
+import { getViteErrorPayload } from '../errors/dev/vite.js';
import type { ModuleLoader, ModuleLoaderEventEmitter } from './loader.js';
export function createViteLoader(viteServer: vite.ViteDevServer): ModuleLoader {
@@ -43,6 +46,24 @@ export function createViteLoader(viteServer: vite.ViteDevServer): ModuleLoader {
}
const msg = args[0] as vite.HMRPayload;
if (msg?.type === 'error') {
+ // If we have an error, but it didn't go through our error enhancement program, it means that it's a HMR error from
+ // vite itself, which goes through a different path. We need to enhance it here.
+ if (!(msg as any)['__isEnhancedAstroErrorPayload']) {
+ const err = collectErrorMetadata(msg.err, pathToFileURL(viteServer.config.root));
+ getViteErrorPayload(err).then((payload) => {
+ events.emit('hmr-error', {
+ type: 'error',
+ err: {
+ message: payload.err.message,
+ stack: payload.err.stack,
+ },
+ });
+
+ args[0] = payload;
+ _wsSend.apply(this, args);
+ });
+ return;
+ }
events.emit('hmr-error', msg);
}
_wsSend.apply(this, args);
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
index 459d24cc3..7c2a5bd9a 100644
--- a/packages/astro/src/vite-plugin-astro-server/route.ts
+++ b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -1,6 +1,7 @@
import type http from 'node:http';
import {
DEFAULT_404_COMPONENT,
+ NOOP_MIDDLEWARE_HEADER,
REROUTE_DIRECTIVE_HEADER,
REWRITE_DIRECTIVE_HEADER_KEY,
clientLocalsSymbol,
@@ -134,7 +135,6 @@ type HandleRoute = {
manifestData: ManifestData;
incomingRequest: http.IncomingMessage;
incomingResponse: http.ServerResponse;
- status?: 404 | 500 | 200;
pipeline: DevPipeline;
};
@@ -142,7 +142,6 @@ export async function handleRoute({
matchedRoute,
url,
pathname,
- status = getStatus(matchedRoute),
body,
origin,
pipeline,
@@ -197,27 +196,32 @@ export async function handleRoute({
mod = preloadedComponent;
- const isDefaultPrerendered404 =
- matchedRoute.route.route === '/404' &&
- matchedRoute.route.prerender &&
- matchedRoute.route.component === DEFAULT_404_COMPONENT;
-
renderContext = await RenderContext.create({
locals,
pipeline,
pathname,
- middleware: isDefaultPrerendered404 ? undefined : middleware,
+ middleware: isDefaultPrerendered404(matchedRoute.route) ? undefined : middleware,
request,
routeData: route,
});
let response;
+ let statusCode = 200;
let isReroute = false;
let isRewrite = false;
try {
response = await renderContext.render(mod);
isReroute = response.headers.has(REROUTE_DIRECTIVE_HEADER);
isRewrite = response.headers.has(REWRITE_DIRECTIVE_HEADER_KEY);
+ const statusCodedMatched = getStatusByMatchedRoute(matchedRoute);
+ statusCode = isRewrite
+ ? // Ignore `matchedRoute` status for rewrites
+ response.status
+ : // Our internal noop middleware sets a particular header. If the header isn't present, it means that the user have
+ // their own middleware, so we need to return what the user returns.
+ !response.headers.has(NOOP_MIDDLEWARE_HEADER) && !isReroute
+ ? response.status
+ : (statusCodedMatched ?? response.status);
} catch (err: any) {
const custom500 = getCustom500Route(manifestData);
if (!custom500) {
@@ -229,7 +233,7 @@ export async function handleRoute({
const preloaded500Component = await pipeline.preload(custom500, filePath500);
renderContext.props.error = err;
response = await renderContext.render(preloaded500Component);
- status = 500;
+ statusCode = 500;
}
if (isLoggedRequest(pathname)) {
@@ -239,20 +243,20 @@ export async function handleRoute({
req({
url: pathname,
method: incomingRequest.method,
- statusCode: isRewrite ? response.status : (status ?? response.status),
+ statusCode,
isRewrite,
reqTime: timeEnd - timeStart,
}),
);
}
- if (response.status === 404 && response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no') {
+
+ if (statusCode === 404 && response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no') {
const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline);
if (options && options.route !== fourOhFourRoute?.route)
return handleRoute({
...options,
matchedRoute: fourOhFourRoute,
url: new URL(pathname, url),
- status: 404,
body,
origin,
pipeline,
@@ -318,18 +322,22 @@ export async function handleRoute({
// Apply the `status` override to the response object before responding.
// Response.status is read-only, so a clone is required to override.
- if (status && response.status !== status && (status === 404 || status === 500)) {
+ if (response.status !== statusCode) {
response = new Response(response.body, {
- status: status,
+ status: statusCode,
headers: response.headers,
});
}
await writeSSRResult(request, response, incomingResponse);
}
-function getStatus(matchedRoute?: MatchedRoute): 404 | 500 | 200 {
- if (!matchedRoute) return 404;
- if (matchedRoute.route.route === '/404') return 404;
- if (matchedRoute.route.route === '/500') return 500;
- return 200;
+/** Check for /404 and /500 custom routes to compute status code */
+function getStatusByMatchedRoute(matchedRoute?: MatchedRoute) {
+ if (matchedRoute?.route.route === '/404') return 404;
+ if (matchedRoute?.route.route === '/500') return 500;
+ return undefined;
+}
+
+function isDefaultPrerendered404(route: RouteData) {
+ return route.route === '/404' && route.prerender && route.component === DEFAULT_404_COMPONENT;
}
diff --git a/packages/astro/test/actions.test.js b/packages/astro/test/actions.test.js
index c34b91a7d..257bf0284 100644
--- a/packages/astro/test/actions.test.js
+++ b/packages/astro/test/actions.test.js
@@ -132,6 +132,12 @@ describe('Astro Actions', () => {
assert.equal(data, 'Hello, ben!');
}
});
+
+ it('Should fail when calling an action without using Astro.callAction', async () => {
+ const res = await fixture.fetch('/invalid/');
+ const text = await res.text();
+ assert.match(text, /ActionCalledFromServerError/);
+ });
});
describe('build', () => {
diff --git a/packages/astro/test/fixtures/actions/src/pages/invalid.astro b/packages/astro/test/fixtures/actions/src/pages/invalid.astro
new file mode 100644
index 000000000..908eee853
--- /dev/null
+++ b/packages/astro/test/fixtures/actions/src/pages/invalid.astro
@@ -0,0 +1,6 @@
+---
+import { actions } from "astro:actions";
+
+// this is invalid, it should fail
+const result = await actions.imageUploadInChunks();
+---
diff --git a/packages/astro/test/fixtures/middleware space/src/middleware.js b/packages/astro/test/fixtures/middleware space/src/middleware.js
index ed0503c80..ac9ffba2f 100644
--- a/packages/astro/test/fixtures/middleware space/src/middleware.js
+++ b/packages/astro/test/fixtures/middleware space/src/middleware.js
@@ -74,4 +74,13 @@ const third = defineMiddleware(async (context, next) => {
return next();
});
-export const onRequest = sequence(first, second, third);
+const fourth = defineMiddleware((context, next) => {
+ if (context.request.url.includes('/no-route-but-200')) {
+ return new Response("It's OK!", {
+ status: 200
+ });
+ }
+ return next()
+})
+
+export const onRequest = sequence(first, second, third, fourth);
diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js
index 63c2788bc..26bc06d77 100644
--- a/packages/astro/test/middleware.test.js
+++ b/packages/astro/test/middleware.test.js
@@ -70,6 +70,13 @@ describe('Middleware in DEV mode', () => {
assert.equal($('title').html(), 'MiddlewareNoDataOrNextCalled');
});
+ it('should return 200 if the middleware returns a 200 Response', async () => {
+ const response = await fixture.fetch('/no-route-but-200');
+ assert.equal(response.status, 200);
+ const html = await response.text();
+ assert.match(html, /It's OK!/);
+ });
+
it('should allow setting cookies', async () => {
const res = await fixture.fetch('/');
assert.equal(res.headers.get('set-cookie'), 'foo=bar');
@@ -245,6 +252,14 @@ describe('Middleware API in PROD mode, SSR', () => {
assert.notEqual($('title').html(), 'MiddlewareNoDataReturned');
});
+ it('should return 200 if the middleware returns a 200 Response', async () => {
+ const request = new Request('http://example.com/no-route-but-200');
+ const response = await app.render(request);
+ assert.equal(response.status, 200);
+ const html = await response.text();
+ assert.match(html, /It's OK!/);
+ });
+
it('should correctly work for API endpoints that return a Response object', async () => {
const request = new Request('http://example.com/api/endpoint');
const response = await app.render(request);