Adding an option to disable HTTP streaming (#3777)

* Adding a flag to disable HTTP streaming * refactor: adding support for SSG builds * handling string responses in the server runtime, adding tests * removing streaming CLI flag * removing import.meta.env.STREAMING * include Content-Length header when streaming is disabled * Verifying content-length header in dev * fix: default streaming to enabled in the base App server * TEMP: disabling the production test to investigate the test-adapter * re-enabling the test with an adapter option to disable streaming for the test * fix: use the existing TextEncoder to get the body's byte length * moving config to build.streaming, ignoring it in `dev` * fixing dev test to expect response streaming * chore: add changsets * removing the new config option all together 🎉 * remove temp debug log * Updating astro changeset now that streaming isn't a config option
withastro · Jul 1, 2022 · 976e1f1 · 976e1f1
1 parent f9290b3
commit 976e1f1
Show file tree

Hide file tree

Showing 16 changed files with 155 additions and 26 deletions.
diff --git a/.changeset/curly-worms-agree.md b/.changeset/curly-worms-agree.md
@@ -0,0 +1,5 @@
+---
+'@astrojs/cloudflare': patch
+---
+
+Disables HTTP streaming in Cloudflare Pages deployments
diff --git a/.changeset/two-dolls-marry.md b/.changeset/two-dolls-marry.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Adds an option to disable HTTP streaming in Astro's production `App` server
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
@@ -34,14 +34,16 @@ export class App {
 		dest: consoleLogDestination,
 		level: 'info',
 	};
+	#streaming: boolean;
 
-	constructor(manifest: Manifest) {
+	constructor(manifest: Manifest, streaming = true) {
 		this.#manifest = manifest;
 		this.#manifestData = {
 			routes: manifest.routes.map((route) => route.routeData),
 		};
 		this.#routeDataToRouteInfo = new Map(manifest.routes.map((route) => [route.routeData, route]));
 		this.#routeCache = new RouteCache(this.#logging);
+		this.#streaming = streaming;
 	}
 	match(request: Request): RouteData | undefined {
 		const url = new URL(request.url);
@@ -117,6 +119,7 @@ export class App {
 			site: this.#manifest.site,
 			ssr: true,
 			request,
+			streaming: this.#streaming,
 		});
 
 		return response;

diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -240,6 +240,7 @@ async function generatePath(
 			? new URL(astroConfig.base, astroConfig.site).toString()
 			: astroConfig.site,
 		ssr,
+		streaming: true,
 	};
 
 	let body: string;

diff --git a/packages/astro/src/core/config.ts b/packages/astro/src/core/config.ts
@@ -34,6 +34,7 @@ const ASTRO_CONFIG_DEFAULTS: AstroUserConfig & any = {
 	server: {
 		host: false,
 		port: 3000,
+		streaming: true,
 	},
 	style: { postcss: { options: {}, plugins: [] } },
 	integrations: [],
@@ -315,6 +316,7 @@ export async function validateConfig(
 						.optional()
 						.default(ASTRO_CONFIG_DEFAULTS.server.host),
 					port: z.number().optional().default(ASTRO_CONFIG_DEFAULTS.server.port),
+					streaming: z.boolean().optional().default(true),
 				})
 				.optional()
 				.default({})

diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts
@@ -80,6 +80,7 @@ export interface RenderOptions {
 	routeCache: RouteCache;
 	site?: string;
 	ssr: boolean;
+	streaming: boolean;
 	request: Request;
 }
 
@@ -100,6 +101,7 @@ export async function render(opts: RenderOptions): Promise<Response> {
 		routeCache,
 		site,
 		ssr,
+		streaming,
 	} = opts;
 
 	const paramsAndPropsRes = await getParamsAndProps({
@@ -138,12 +140,13 @@ export async function render(opts: RenderOptions): Promise<Response> {
 		site,
 		scripts,
 		ssr,
+		streaming,
 	});
 
 	// Support `export const components` for `MDX` pages
 	if (typeof (mod as any).components === 'object') {
 		Object.assign(pageProps, { components: (mod as any).components });
 	}
 
-	return await renderPage(result, Component, pageProps, null);
+	return await renderPage(result, Component, pageProps, null, streaming);
 }
diff --git a/packages/astro/src/core/render/dev/index.ts b/packages/astro/src/core/render/dev/index.ts
@@ -184,6 +184,7 @@ export async function render(
 		routeCache,
 		site: astroConfig.site ? new URL(astroConfig.base, astroConfig.site).toString() : undefined,
 		ssr: isBuildingToSSR(astroConfig),
+		streaming: true,
 	});
 
 	return response;

diff --git a/packages/astro/src/core/render/result.ts b/packages/astro/src/core/render/result.ts
@@ -24,6 +24,7 @@ function onlyAvailableInSSR(name: string) {
 
 export interface CreateResultArgs {
 	ssr: boolean;
+	streaming: boolean;
 	logging: LogOptions;
 	origin: string;
 	markdown: MarkdownRenderingOptions;
@@ -114,7 +115,11 @@ export function createResult(args: CreateResultArgs): SSRResult {
 	const url = new URL(request.url);
 	const canonicalURL = createCanonicalURL('.' + pathname, site ?? url.origin, paginated);
 	const headers = new Headers();
-	headers.set('Transfer-Encoding', 'chunked');
+	if (args.streaming) {
+		headers.set('Transfer-Encoding', 'chunked');
+	} else {
+		headers.set('Content-Type', 'text/html');
+	}
 	const response: ResponseInit = {
 		status: 200,
 		statusText: 'OK',

diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts
@@ -704,7 +704,8 @@ export async function renderPage(
 	result: SSRResult,
 	componentFactory: AstroComponentFactory,
 	props: any,
-	children: any
+	children: any,
+	streaming: boolean,
 ): Promise<Response> {
 	let iterable: AsyncIterable<any>;
 	if (!componentFactory.isAstroComponentFactory) {
@@ -730,28 +731,48 @@ export async function renderPage(
 	const factoryReturnValue = await componentFactory(result, props, children);
 
 	if (isAstroComponent(factoryReturnValue)) {
-		iterable = renderAstroComponent(factoryReturnValue);
-		let stream = new ReadableStream({
-			start(controller) {
-				async function read() {
-					let i = 0;
-					for await (const chunk of iterable) {
-						let html = chunk.toString();
-						if (i === 0) {
-							if (!/<!doctype html/i.test(html)) {
-								controller.enqueue(encoder.encode('<!DOCTYPE html>\n'));
+		let iterable = renderAstroComponent(factoryReturnValue);
+		let init = result.response;
+		let headers = new Headers(init.headers);
+		let body: BodyInit;
+		if (streaming) {
+			body = new ReadableStream({
+				start(controller) {
+					async function read() {
+						let i = 0;
+						for await (const chunk of iterable) {
+							let html = chunk.toString();
+							if (i === 0) {
+								if (!/<!doctype html/i.test(html)) {
+									controller.enqueue(encoder.encode('<!DOCTYPE html>\n'));
+								}
 							}
+							controller.enqueue(encoder.encode(html));
+							i++;
 						}
-						controller.enqueue(encoder.encode(html));
-						i++;
+						controller.close();
+					}
+					read();
+				},
+			});
+		} else {
+		  body = '';
+			let i = 0;
+			for await (const chunk of iterable) {
+				let html = chunk.toString();
+				if (i === 0) {
+					if (!/<!doctype html/i.test(html)) {
+						body += '<!DOCTYPE html>\n';
 					}
-					controller.close();
 				}
-				read();
-			},
-		});
-		let init = result.response;
-		let response = createResponse(stream, init);
+				body += chunk;
+				i++;
+			}
+			const bytes = encoder.encode(body);
+			headers.set('Content-Length', `${bytes.byteLength}`);
+		}
+
+		let response = createResponse(body, { ...init, headers });
 		return response;
 	} else {
 		return factoryReturnValue;

diff --git a/packages/astro/src/runtime/server/response.ts b/packages/astro/src/runtime/server/response.ts
@@ -51,6 +51,9 @@ type CreateResponseFn = (body?: BodyInit | null, init?: ResponseInit) => Respons
 
 export const createResponse: CreateResponseFn = isNodeJS
 	? (body, init) => {
+			if (typeof body === 'string') {
+				return new Response(body, init);
+			}
 			if (typeof StreamingCompatibleResponse === 'undefined') {
 				return new (createResponseClass())(body, init);
 			}

diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts
@@ -84,6 +84,8 @@ async function writeWebResponse(res: http.ServerResponse, webResponse: Response)
 		} else if (body instanceof Readable) {
 			body.pipe(res);
 			return;
+		} else if (typeof body === 'string') {
+			res.write(body);
 		} else {
 			const reader = body.getReader();
 			while (true) {

diff --git a/packages/astro/test/streaming.test.js b/packages/astro/test/streaming.test.js
@@ -71,3 +71,72 @@ describe('Streaming', () => {
 		});
 	});
 });
+
+describe('Streaming disabled', () => {
+	if (isWindows) return;
+
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/streaming/',
+			adapter: testAdapter(),
+			experimental: {
+				ssr: true,
+			},
+			server: {
+				streaming: false,
+			}
+		});
+	});
+
+	describe('Development', () => {
+		/** @type {import('./test-utils').DevServer} */
+		let devServer;
+
+		before(async () => {
+			devServer = await fixture.startDevServer();
+		});
+
+		after(async () => {
+			await devServer.stop();
+		});
+
+		it('Body is chunked', async () => {
+			let res = await fixture.fetch('/');
+			let chunks = [];
+			for await (const bytes of res.body) {
+				let chunk = bytes.toString('utf-8');
+				chunks.push(chunk);
+			}
+			expect(chunks.length).to.be.greaterThan(1);
+		});
+	});
+
+	// TODO: find a different solution for the test-adapter,
+	// currently there's no way to resolve two different versions with one
+	// having streaming disabled
+	describe('Production', () => {
+		before(async () => {
+			await fixture.build();
+		});
+
+		it('Can get the full html body', async () => {
+			const app = await fixture.loadTestAdapterApp(false);
+			const request = new Request('http://example.com/');
+			const response = await app.render(request);
+
+			expect(response.status).to.equal(200);
+			expect(response.headers.get('content-type')).to.equal('text/html');
+			expect(response.headers.has('content-length')).to.equal(true);
+			expect(parseInt(response.headers.get('content-length'))).to.be.greaterThan(0);
+
+			const html = await response.text();
+			const $ = cheerio.load(html);
+
+			expect($('header h1')).to.have.a.lengthOf(1);
+			expect($('ul li')).to.have.a.lengthOf(10);
+		});
+	});
+});
diff --git a/packages/astro/test/test-adapter.js b/packages/astro/test/test-adapter.js
@@ -23,7 +23,7 @@ export default function () {
 								},
 								load(id) {
 									if (id === '@my-ssr') {
-										return `import { App } from 'astro/app';export function createExports(manifest) { return { manifest, createApp: () => new App(manifest) }; }`;
+										return `import { App } from 'astro/app';export function createExports(manifest) { return { manifest, createApp: (streaming) => new App(manifest, streaming) }; }`;
 									}
 								},
 							},

diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js
@@ -149,10 +149,10 @@ export async function loadFixture(inlineConfig) {
 		clean: async () => {
 			await fs.promises.rm(config.outDir, { maxRetries: 10, recursive: true, force: true });
 		},
-		loadTestAdapterApp: async () => {
+		loadTestAdapterApp: async (streaming) => {
 			const url = new URL('./server/entry.mjs', config.outDir);
 			const { createApp, manifest } = await import(url);
-			const app = createApp();
+			const app = createApp(streaming);
 			app.manifest = manifest;
 			return app;
 		},

diff --git a/packages/integrations/cloudflare/src/server.ts b/packages/integrations/cloudflare/src/server.ts
@@ -8,7 +8,7 @@ type Env = {
 };
 
 export function createExports(manifest: SSRManifest) {
-	const app = new App(manifest);
+	const app = new App(manifest, false);
 
 	const fetch = async (request: Request, env: Env) => {
 		const { origin, pathname } = new URL(request.url);

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml