Skip to content

Commit

Permalink
refactor: remove endpoint handling infra (#9775)
Browse files Browse the repository at this point in the history
* remove endpoint handling infra

* add changeset
  • Loading branch information
lilnasy authored Jan 23, 2024
1 parent 9aa7a53 commit 075706f
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 91 deletions.
5 changes: 5 additions & 0 deletions .changeset/tall-comics-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Simplifies internals that handle endpoints.
29 changes: 14 additions & 15 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
createStylesheetElementSet,
} from '../render/ssr-element.js';
import { matchRoute } from '../routing/match.js';
import { EndpointNotFoundError, SSRRoutePipeline } from './ssrPipeline.js';
import { SSRRoutePipeline } from './ssrPipeline.js';
import type { RouteInfo } from './types.js';
export { deserializeManifest } from './common.js';

Expand Down Expand Up @@ -272,28 +272,27 @@ export class App {
}
response = await this.#pipeline.renderRoute(renderContext, pageModule);
} catch (err: any) {
if (err instanceof EndpointNotFoundError) {
return this.#renderError(request, { status: 404, response: err.originalResponse });
} else {
this.#logger.error(null, err.stack || err.message || String(err));
return this.#renderError(request, { status: 500 });
}
this.#logger.error(null, err.stack || err.message || String(err));
return this.#renderError(request, { status: 500 });
}

// endpoints do not participate in implicit rerouting
if (routeData.type === 'page' || routeData.type === 'redirect') {
if (REROUTABLE_STATUS_CODES.has(response.status)) {
return this.#renderError(request, {
response,
status: response.status as 404 | 500,
});
}
if (REROUTABLE_STATUS_CODES.has(response.status) && response.headers.get("X-Astro-Reroute") !== "no") {
return this.#renderError(request, {
response,
status: response.status as 404 | 500,
});
}

if (response.headers.has("X-Astro-Reroute")) {
response.headers.delete("X-Astro-Reroute");
}

if (addCookieHeader) {
for (const setCookieHeaderValue of App.getSetCookieFromResponse(response)) {
response.headers.append('set-cookie', setCookieHeaderValue);
}
}

Reflect.set(response, responseSentSymbol, true);
return response;
}
Expand Down
27 changes: 1 addition & 26 deletions packages/astro/src/core/app/ssrPipeline.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,3 @@
import { Pipeline } from '../pipeline.js';
import type { Environment } from '../render/index.js';

/**
* Thrown when an endpoint contains a response with the header "X-Astro-Response" === 'Not-Found'
*/
export class EndpointNotFoundError extends Error {
originalResponse: Response;
constructor(originalResponse: Response) {
super();
this.originalResponse = originalResponse;
}
}

export class SSRRoutePipeline extends Pipeline {
constructor(env: Environment) {
super(env);
this.setEndpointHandler(this.#ssrEndpointHandler);
}

// This function is responsible for handling the result coming from an endpoint.
async #ssrEndpointHandler(request: Request, response: Response): Promise<Response> {
if (response.headers.get('X-Astro-Response') === 'Not-Found') {
throw new EndpointNotFoundError(response);
}
return response;
}
}
export class SSRRoutePipeline extends Pipeline {}
5 changes: 0 additions & 5 deletions packages/astro/src/core/build/buildPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export class BuildPipeline extends Pipeline {
this.#internals = internals;
this.#staticBuildOptions = staticBuildOptions;
this.#manifest = manifest;
this.setEndpointHandler(this.#handleEndpointResult);
}

getInternals(): Readonly<BuildInternals> {
Expand Down Expand Up @@ -193,8 +192,4 @@ export class BuildPipeline extends Pipeline {

return pages;
}

async #handleEndpointResult(_: Request, response: Response): Promise<Response> {
return response;
}
}
3 changes: 2 additions & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@ async function generatePath(
) {
const { mod, scripts: hoistedScripts, styles: _styles } = gopts;
const manifest = pipeline.getManifest();
pipeline.getEnvironment().logger.debug('build', `Generating: ${pathname}`);
const logger = pipeline.getLogger();
logger.debug('build', `Generating: ${pathname}`);

const links = new Set<never>();
const scripts = createModuleScriptsSet(
Expand Down
30 changes: 1 addition & 29 deletions packages/astro/src/core/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ import { callMiddleware } from './middleware/callMiddleware.js';
import { renderPage } from './render/core.js';
import { type Environment, type RenderContext } from './render/index.js';

type EndpointResultHandler = (
originalRequest: Request,
result: Response
) => Promise<Response> | Response;

type PipelineHooks = {
before: PipelineHookFunction[];
Expand All @@ -26,11 +22,6 @@ export class Pipeline {
#hooks: PipelineHooks = {
before: [],
};
/**
* The handler accepts the *original* `Request` and result returned by the endpoint.
* It must return a `Response`.
*/
#endpointHandler?: EndpointResultHandler;

/**
* When creating a pipeline, an environment is mandatory.
Expand All @@ -42,15 +33,6 @@ export class Pipeline {

setEnvironment() {}

/**
* When rendering a route, an "endpoint" will a type that needs to be handled and transformed into a `Response`.
*
* Each consumer might have different needs; use this function to set up the handler.
*/
setEndpointHandler(handler: EndpointResultHandler) {
this.#endpointHandler = handler;
}

/**
* A middleware function that will be called before each request.
*/
Expand Down Expand Up @@ -81,22 +63,12 @@ export class Pipeline {
for (const hook of this.#hooks.before) {
hook(renderContext, componentInstance);
}
const result = await this.#tryRenderRoute(
return await this.#tryRenderRoute(
renderContext,
this.env,
componentInstance,
this.#onRequest
);
if (renderContext.route.type === 'endpoint') {
if (!this.#endpointHandler) {
throw new Error(
'You created a pipeline that does not know how to handle the result coming from an endpoint.'
);
}
return this.#endpointHandler(renderContext.request, result);
} else {
return result;
}
}

/**
Expand Down
18 changes: 8 additions & 10 deletions packages/astro/src/runtime/server/endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,14 @@ export async function renderEndpoint(
? `One of the exported handlers is "all" (lowercase), did you mean to export 'ALL'?\n`
: '')
);
// No handler found, so this should be a 404. Using a custom header
// to signal to the renderer that this is an internal 404 that should
// be handled by a custom 404 route if possible.
return new Response(null, {
status: 404,
headers: {
'X-Astro-Response': 'Not-Found',
},
});
// No handler matching the verb found, so this should be a
// 404. Should be handled by 404.astro route if possible.
return new Response(null, { status: 404 });
}

return handler.call(mod, context);
const response = await handler.call(mod, context);
// Endpoints explicitly returning 404 or 500 response status should
// NOT be subject to rerouting to 404.astro or 500.astro.
response.headers.set("X-Astro-Reroute", "no");
return response;
}
5 changes: 0 additions & 5 deletions packages/astro/src/vite-plugin-astro-server/devPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export default class DevPipeline extends Pipeline {
this.#devLogger = logger;
this.#settings = settings;
this.#loader = loader;
this.setEndpointHandler(this.#handleEndpointResult);
}

clearRouteCache() {
Expand Down Expand Up @@ -88,9 +87,5 @@ export default class DevPipeline extends Pipeline {
});
}

async #handleEndpointResult(_: Request, response: Response): Promise<Response> {
return response;
}

async handleFallback() {}
}

0 comments on commit 075706f

Please sign in to comment.