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:https://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