From 976e1f175a95ea39f737b8575e4fdf3c3d89e1ee Mon Sep 17 00:00:00 2001 From: Tony Sullivan Date: Fri, 1 Jul 2022 02:29:59 +0000 Subject: [PATCH] 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 :tada: * remove temp debug log * Updating astro changeset now that streaming isn't a config option --- .changeset/curly-worms-agree.md | 5 ++ .changeset/two-dolls-marry.md | 5 ++ packages/astro/src/core/app/index.ts | 5 +- packages/astro/src/core/build/generate.ts | 1 + packages/astro/src/core/config.ts | 2 + packages/astro/src/core/render/core.ts | 5 +- packages/astro/src/core/render/dev/index.ts | 1 + packages/astro/src/core/render/result.ts | 7 +- packages/astro/src/runtime/server/index.ts | 65 +++++++++++------ packages/astro/src/runtime/server/response.ts | 3 + .../src/vite-plugin-astro-server/index.ts | 2 + packages/astro/test/streaming.test.js | 69 +++++++++++++++++++ packages/astro/test/test-adapter.js | 2 +- packages/astro/test/test-utils.js | 4 +- .../integrations/cloudflare/src/server.ts | 2 +- pnpm-lock.yaml | 9 +++ 16 files changed, 158 insertions(+), 29 deletions(-) create mode 100644 .changeset/curly-worms-agree.md create mode 100644 .changeset/two-dolls-marry.md diff --git a/.changeset/curly-worms-agree.md b/.changeset/curly-worms-agree.md new file mode 100644 index 000000000..cd51e1d60 --- /dev/null +++ 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 new file mode 100644 index 000000000..1548dfec9 --- /dev/null +++ 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 index 40842a7f9..6fcb51717 100644 --- 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 index bfc95cb8e..1f88f6ce6 100644 --- 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 index 0fb41032b..f0b77e830 100644 --- 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 index c4d05ff46..1abc89363 100644 --- 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 { routeCache, site, ssr, + streaming, } = opts; const paramsAndPropsRes = await getParamsAndProps({ @@ -138,6 +140,7 @@ export async function render(opts: RenderOptions): Promise { site, scripts, ssr, + streaming, }); // Support `export const components` for `MDX` pages @@ -145,5 +148,5 @@ export async function render(opts: RenderOptions): Promise { 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 index 317202a7b..bd6b3e9a5 100644 --- 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 index cfbc48521..e754b334a 100644 --- 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 index 0d660fc94..55cc23101 100644 --- 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 { let iterable: AsyncIterable; 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 (!/\n')); - } - } - controller.enqueue(encoder.encode(html)); - i++; - } - controller.close(); - } - read(); - }, - }); + let iterable = renderAstroComponent(factoryReturnValue); let init = result.response; - let response = createResponse(stream, init); + 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 (!/\n')); + } + } + 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 (!/\n'; + } + } + 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 index 1e92c47ba..184d00a32 100644 --- 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 index a57e2bba0..b3bbd9726 100644 --- 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 index 7d28387d3..266853787 100644 --- 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 index 0ed8014ce..4b7eac527 100644 --- 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 index 94abd6ef4..346c2a2ac 100644 --- 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 index 032146691..8e7f572c6 100644 --- 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 index 564a4b91b..072d4b3dc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8905,6 +8905,11 @@ packages: /debug/3.2.7: resolution: {integrity: sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==} + peerDependencies: + supports-color: '*' + peerDependenciesMeta: + supports-color: + optional: true dependencies: ms: 2.1.3 dev: false @@ -11957,6 +11962,8 @@ packages: debug: 3.2.7 iconv-lite: 0.4.24 sax: 1.2.4 + transitivePeerDependencies: + - supports-color dev: false /netmask/2.0.2: @@ -12040,6 +12047,8 @@ packages: rimraf: 2.7.1 semver: 5.7.1 tar: 4.4.19 + transitivePeerDependencies: + - supports-color dev: false /node-releases/2.0.5: