From 5dc0de10367fc215dfe7dbab34a768cb893f1cd5 Mon Sep 17 00:00:00 2001 From: hbgl Date: Sat, 8 Jul 2023 01:08:45 +0200 Subject: [PATCH 1/9] Draft: support request body streaming. Tests are broken because node-mocks-http does not support the async iterable iterface for IncomingRequest. --- packages/astro/src/core/app/node.ts | 135 +++++++++++++++++----------- 1 file changed, 84 insertions(+), 51 deletions(-) diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 3e620730959f..6e9e0d8a94ec 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -9,20 +9,33 @@ import { App, type MatchOptions } from './index.js'; const clientAddressSymbol = Symbol.for('astro.clientAddress'); -function createRequestFromNodeRequest(req: NodeIncomingMessage, body?: Uint8Array): Request { +type CreateNodeRequestOptions = { + emptyBody?: boolean; +}; + +type BodyProps = Partial; + +function createRequestFromNodeRequest( + req: NodeIncomingMessage, + options?: CreateNodeRequestOptions +): Request { const protocol = req.socket instanceof TLSSocket || req.headers['x-forwarded-proto'] === 'https' ? 'https' : 'http'; const hostname = req.headers.host || req.headers[':authority']; const url = `${protocol}://${hostname}${req.url}`; - const rawHeaders = req.headers as Record; - const entries = Object.entries(rawHeaders); + const headers = makeRequestHeaders(req); const method = req.method || 'GET'; + let bodyProps: BodyProps = {}; + const bodyAllowed = method !== 'HEAD' && method !== 'GET' && !options?.emptyBody; + if (bodyAllowed) { + bodyProps = makeRequestBody(req); + } const request = new Request(url, { method, - headers: new Headers(entries), - body: ['HEAD', 'GET'].includes(method) ? null : body, + headers, + ...bodyProps, }); if (req.socket?.remoteAddress) { Reflect.set(request, clientAddressSymbol, req.socket.remoteAddress); @@ -30,63 +43,83 @@ function createRequestFromNodeRequest(req: NodeIncomingMessage, body?: Uint8Arra return request; } -class NodeIncomingMessage extends IncomingMessage { - /** - * The read-only body property of the Request interface contains a ReadableStream with the body contents that have been added to the request. - */ - body?: unknown; +function makeRequestHeaders(req: NodeIncomingMessage): Headers { + const headers = new Headers(); + for (const [name, value] of Object.entries(req.headers)) { + if (value === undefined) { + continue; + } + if (Array.isArray(value)) { + for (const item of value) { + headers.append(name, item); + } + } else { + headers.append(name, value); + } + } + return headers; } -export class NodeApp extends App { - match(req: NodeIncomingMessage | Request, opts: MatchOptions = {}) { - return super.match(req instanceof Request ? req : createRequestFromNodeRequest(req), opts); - } - render(req: NodeIncomingMessage | Request, routeData?: RouteData, locals?: object) { +function makeRequestBody(req: NodeIncomingMessage): BodyProps { + if (req.body !== undefined) { if (typeof req.body === 'string' && req.body.length > 0) { - return super.render( - req instanceof Request ? req : createRequestFromNodeRequest(req, Buffer.from(req.body)), - routeData, - locals - ); + return { body: Buffer.from(req.body) }; } if (typeof req.body === 'object' && req.body !== null && Object.keys(req.body).length > 0) { - return super.render( - req instanceof Request - ? req - : createRequestFromNodeRequest(req, Buffer.from(JSON.stringify(req.body))), - routeData, - locals - ); + return { body: Buffer.from(JSON.stringify(req.body)) }; } - if ('on' in req) { - let body = Buffer.from([]); - let reqBodyComplete = new Promise((resolve, reject) => { - req.on('data', (d) => { - body = Buffer.concat([body, d]); - }); - req.on('end', () => { - resolve(body); - }); - req.on('error', (err) => { - reject(err); - }); - }); + // This covers all async iterables including Readable and ReadableStream. + if ( + typeof req.body === 'object' && + req.body !== null && + typeof (req.body as any)[Symbol.asyncIterator] !== 'undefined' + ) { + return asyncIterableToBodyProps(req.body as AsyncIterable); + } + } + + return asyncIterableToBodyProps(req); +} + +function asyncIterableToBodyProps(iterable: AsyncIterable): BodyProps { + // Return default body. + return { + // Node uses undici for the Request implementation. Undici accepts + // a non-standard async iterables for the body. + // @ts-ignore + body: iterable, + // The duplex property is required when using a ReadableStream or async + // iterable for the body. The type definitions do not include the duplex + // property because they are not up-to-date. + // @ts-ignore + duplex: 'half', + } satisfies BodyProps; +} + +class NodeIncomingMessage extends IncomingMessage { + /** + * Allow the request body to be explicitly overridden. For example, this + * is used by the Express JSON middleware. + */ + body?: unknown; +} - return reqBodyComplete.then(() => { - return super.render( - req instanceof Request ? req : createRequestFromNodeRequest(req, body), - routeData, - locals - ); +export class NodeApp extends App { + match(req: NodeIncomingMessage | Request, opts: MatchOptions = {}) { + if (!(req instanceof Request)) { + req = createRequestFromNodeRequest(req, { + emptyBody: true, }); } - return super.render( - req instanceof Request ? req : createRequestFromNodeRequest(req), - routeData, - locals - ); + return super.match(req, opts); + } + render(req: NodeIncomingMessage | Request, routeData?: RouteData, locals?: object) { + if (!(req instanceof Request)) { + req = createRequestFromNodeRequest(req); + } + return super.render(req, routeData, locals); } } From 948bf52d14e6b3cbb83eaad46458525020204c54 Mon Sep 17 00:00:00 2001 From: hbgl Date: Sat, 8 Jul 2023 01:18:45 +0200 Subject: [PATCH 2/9] Fix comment placement --- packages/astro/src/core/app/node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 6e9e0d8a94ec..ebfee07c1760 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -80,11 +80,11 @@ function makeRequestBody(req: NodeIncomingMessage): BodyProps { } } + // Return default body. return asyncIterableToBodyProps(req); } function asyncIterableToBodyProps(iterable: AsyncIterable): BodyProps { - // Return default body. return { // Node uses undici for the Request implementation. Undici accepts // a non-standard async iterables for the body. From 31bc05f5f41d4bbd0ace2ca568073ca7e9925eff Mon Sep 17 00:00:00 2001 From: hbgl Date: Tue, 15 Aug 2023 01:48:48 +0200 Subject: [PATCH 3/9] Fix tests. --- packages/integrations/node/package.json | 2 +- .../integrations/node/test/api-route.test.js | 9 +++++-- pnpm-lock.yaml | 24 ++++++++++++++----- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/integrations/node/package.json b/packages/integrations/node/package.json index 4f8a3021b250..1db4340a7dd6 100644 --- a/packages/integrations/node/package.json +++ b/packages/integrations/node/package.json @@ -49,7 +49,7 @@ "chai": "^4.3.7", "cheerio": "1.0.0-rc.12", "mocha": "^9.2.2", - "node-mocks-http": "^1.12.2", + "node-mocks-http": "^1.13.0", "undici": "^5.22.1" } } diff --git a/packages/integrations/node/test/api-route.test.js b/packages/integrations/node/test/api-route.test.js index 7fbd9577630b..004a9796dd1f 100644 --- a/packages/integrations/node/test/api-route.test.js +++ b/packages/integrations/node/test/api-route.test.js @@ -24,7 +24,9 @@ describe('API routes', () => { handler(req, res); - req.send(JSON.stringify({ id: 2 })); + req.once('async_iterator', () => { + req.send(JSON.stringify({ id: 2 })); + }); let [buffer] = await done; @@ -44,7 +46,10 @@ describe('API routes', () => { }); handler(req, res); - req.send(Buffer.from(new Uint8Array([1, 2, 3, 4, 5]))); + + req.on('async_iterator', () => { + req.send(Buffer.from(new Uint8Array([1, 2, 3, 4, 5]))); + }); let [out] = await done; let arr = Array.from(new Uint8Array(out.buffer)); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 95c239d23eea..c99ef0b8c48d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4523,8 +4523,8 @@ importers: specifier: ^9.2.2 version: 9.2.2 node-mocks-http: - specifier: ^1.12.2 - version: 1.12.2 + specifier: ^1.13.0 + version: 1.13.0 undici: specifier: ^5.22.1 version: 5.22.1 @@ -14300,6 +14300,22 @@ packages: type-is: 1.6.18 dev: true + /node-mocks-http@1.13.0: + resolution: {integrity: sha512-lArD6sJMPJ53WF50GX0nJ89B1nkV1TdMvNwq8WXXFrUXF80ujSyye1T30mgiHh4h2It0/svpF3C4kZ2OAONVlg==} + engines: {node: '>=14'} + dependencies: + accepts: 1.3.8 + content-disposition: 0.5.4 + depd: 1.1.2 + fresh: 0.5.2 + merge-descriptors: 1.0.1 + methods: 1.1.2 + mime: 1.6.0 + parseurl: 1.3.3 + range-parser: 1.2.1 + type-is: 1.6.18 + dev: true + /node-releases@2.0.10: resolution: {integrity: sha512-5GFldHPXVG/YZmFzJvKK2zDSzPKhEp0+ZR5SVaoSag9fsL5YgHbUHDfnG5494ISANDcK4KwPXAx2xqVEydmd7w==} @@ -18226,25 +18242,21 @@ packages: file:packages/astro/test/fixtures/css-assets/packages/font-awesome: resolution: {directory: packages/astro/test/fixtures/css-assets/packages/font-awesome, type: directory} name: '@test/astro-font-awesome-package' - version: 0.0.1 dev: false file:packages/astro/test/fixtures/multiple-renderers/renderers/one: resolution: {directory: packages/astro/test/fixtures/multiple-renderers/renderers/one, type: directory} name: '@test/astro-renderer-one' - version: 1.0.0 dev: false file:packages/astro/test/fixtures/multiple-renderers/renderers/two: resolution: {directory: packages/astro/test/fixtures/multiple-renderers/renderers/two, type: directory} name: '@test/astro-renderer-two' - version: 1.0.0 dev: false file:packages/astro/test/fixtures/solid-component/deps/solid-jsx-component: resolution: {directory: packages/astro/test/fixtures/solid-component/deps/solid-jsx-component, type: directory} name: '@test/solid-jsx-component' - version: 0.0.0 dependencies: solid-js: 1.7.6 dev: false From ba3e84ba6cb019ad987f5444de3f27193fa2238c Mon Sep 17 00:00:00 2001 From: hbgl Date: Tue, 15 Aug 2023 01:56:59 +0200 Subject: [PATCH 4/9] Fix typo. --- packages/astro/src/core/app/node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 73a4b5fafcb1..5c93e7cea4f5 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -87,7 +87,7 @@ function makeRequestBody(req: NodeIncomingMessage): BodyProps { function asyncIterableToBodyProps(iterable: AsyncIterable): BodyProps { return { // Node uses undici for the Request implementation. Undici accepts - // a non-standard async iterables for the body. + // a non-standard async iterable for the body. // @ts-ignore body: iterable, // The duplex property is required when using a ReadableStream or async From e3dc913df4b23827f59cf5e083cc97bea76b90d7 Mon Sep 17 00:00:00 2001 From: hbgl Date: Tue, 15 Aug 2023 02:07:35 +0200 Subject: [PATCH 5/9] Add changeset. --- .changeset/lemon-lobsters-do.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/lemon-lobsters-do.md diff --git a/.changeset/lemon-lobsters-do.md b/.changeset/lemon-lobsters-do.md new file mode 100644 index 000000000000..cfe50300c866 --- /dev/null +++ b/.changeset/lemon-lobsters-do.md @@ -0,0 +1,6 @@ +--- +'@astrojs/node': patch +'astro': patch +--- + +Stream request body instead of buffering it in memory. From 3e9cb7744781184f4b08302e507716ea2ff59a3b Mon Sep 17 00:00:00 2001 From: hbgl Date: Tue, 15 Aug 2023 04:25:54 +0200 Subject: [PATCH 6/9] Add test. Minor test refactoring for clarity. --- .../integrations/node/test/api-route.test.js | 44 ++++++++++++++++--- .../test/fixtures/api-route/src/pages/hash.ts | 16 +++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 packages/integrations/node/test/fixtures/api-route/src/pages/hash.ts diff --git a/packages/integrations/node/test/api-route.test.js b/packages/integrations/node/test/api-route.test.js index 004a9796dd1f..7a0d77c3e718 100644 --- a/packages/integrations/node/test/api-route.test.js +++ b/packages/integrations/node/test/api-route.test.js @@ -1,6 +1,7 @@ import nodejs from '../dist/index.js'; import { loadFixture, createRequestAndResponse } from './test-utils.js'; import { expect } from 'chai'; +import crypto from 'node:crypto'; describe('API routes', () => { /** @type {import('./test-utils').Fixture} */ @@ -22,12 +23,12 @@ describe('API routes', () => { url: '/recipes', }); - handler(req, res); - req.once('async_iterator', () => { req.send(JSON.stringify({ id: 2 })); }); + handler(req, res); + let [buffer] = await done; let json = JSON.parse(buffer.toString('utf-8')); @@ -45,14 +46,47 @@ describe('API routes', () => { url: '/binary', }); - handler(req, res); - - req.on('async_iterator', () => { + req.once('async_iterator', () => { req.send(Buffer.from(new Uint8Array([1, 2, 3, 4, 5]))); }); + handler(req, res); + let [out] = await done; let arr = Array.from(new Uint8Array(out.buffer)); expect(arr).to.deep.equal([5, 4, 3, 2, 1]); }); + + it('Can post large binary daya', async () => { + const { handler } = await import('./fixtures/api-route/dist/server/entry.mjs'); + + let { req, res, done } = createRequestAndResponse({ + method: 'POST', + url: '/hash', + }); + + handler(req, res); + + let expectedDigest = null; + req.once('async_iterator', () => { + // Send 256MB of garbage data in 256KB chunks. This should be fast (< 1sec). + let remainingBytes = 128 * 1024 * 1024; + const chunkSize = 256 * 1024; + + const hash = crypto.createHash('sha256'); + while (remainingBytes > 0) { + const size = Math.min(remainingBytes, chunkSize); + const chunk = Buffer.alloc(size, Math.floor(Math.random() * 256)); + hash.update(chunk); + req.emit('data', chunk); + remainingBytes -= size; + } + + req.emit('end'); + expectedDigest = hash.digest(); + }); + + let [out] = await done; + expect(new Uint8Array(out.buffer)).to.deep.equal(expectedDigest); + }); }); diff --git a/packages/integrations/node/test/fixtures/api-route/src/pages/hash.ts b/packages/integrations/node/test/fixtures/api-route/src/pages/hash.ts new file mode 100644 index 000000000000..fbf44c5478bc --- /dev/null +++ b/packages/integrations/node/test/fixtures/api-route/src/pages/hash.ts @@ -0,0 +1,16 @@ +import crypto from 'node:crypto'; + +export async function post({ request }: { request: Request }) { + const hash = crypto.createHash('sha256'); + + const iterable = request.body as unknown as AsyncIterable; + for await (const chunk of iterable) { + hash.update(chunk); + } + + return new Response(hash.digest(), { + headers: { + 'Content-Type': 'application/octet-stream' + } + }); +} From ad8f356117d0729ae454be64d3b837a848e01b10 Mon Sep 17 00:00:00 2001 From: hbgl Date: Tue, 15 Aug 2023 10:47:34 +0200 Subject: [PATCH 7/9] Fix replace ts-ignore with ts-expect-error. --- packages/astro/src/core/app/node.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 5c93e7cea4f5..4ae6e98a9c20 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -88,12 +88,12 @@ function asyncIterableToBodyProps(iterable: AsyncIterable): BodyProps { return { // Node uses undici for the Request implementation. Undici accepts // a non-standard async iterable for the body. - // @ts-ignore + // @ts-expect-error body: iterable, // The duplex property is required when using a ReadableStream or async // iterable for the body. The type definitions do not include the duplex // property because they are not up-to-date. - // @ts-ignore + // @ts-expect-error duplex: 'half', } satisfies BodyProps; } From 196741b77fdbefd548534ac6b1e78786c91d434c Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 15 Aug 2023 07:58:18 -0400 Subject: [PATCH 8/9] Update packages/integrations/node/test/api-route.test.js --- packages/integrations/node/test/api-route.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integrations/node/test/api-route.test.js b/packages/integrations/node/test/api-route.test.js index 7a0d77c3e718..8ac030229587 100644 --- a/packages/integrations/node/test/api-route.test.js +++ b/packages/integrations/node/test/api-route.test.js @@ -57,7 +57,7 @@ describe('API routes', () => { expect(arr).to.deep.equal([5, 4, 3, 2, 1]); }); - it('Can post large binary daya', async () => { + it('Can post large binary data', async () => { const { handler } = await import('./fixtures/api-route/dist/server/entry.mjs'); let { req, res, done } = createRequestAndResponse({ From 85a28e00cf3b26d10a7cf49abe75302116674f95 Mon Sep 17 00:00:00 2001 From: hbgl Date: Tue, 15 Aug 2023 14:47:35 +0200 Subject: [PATCH 9/9] Make test code match comment. --- packages/integrations/node/test/api-route.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integrations/node/test/api-route.test.js b/packages/integrations/node/test/api-route.test.js index 8ac030229587..c830eee2d6c8 100644 --- a/packages/integrations/node/test/api-route.test.js +++ b/packages/integrations/node/test/api-route.test.js @@ -70,7 +70,7 @@ describe('API routes', () => { let expectedDigest = null; req.once('async_iterator', () => { // Send 256MB of garbage data in 256KB chunks. This should be fast (< 1sec). - let remainingBytes = 128 * 1024 * 1024; + let remainingBytes = 256 * 1024 * 1024; const chunkSize = 256 * 1024; const hash = crypto.createHash('sha256');