diff --git a/.changeset/green-pens-boil.md b/.changeset/green-pens-boil.md new file mode 100644 index 0000000000..b92c4baf8b --- /dev/null +++ b/.changeset/green-pens-boil.md @@ -0,0 +1,9 @@ +--- +'@whatwg-node/fetch': patch +'@whatwg-node/server': patch +--- + +- On Node 14, fix the return method of Response.body's AsyncIterator to close HTTP connection correctly +- On Node 14, handle ReadableStream's cancel correctly if Response.body is a ReadableStream +- Do not modify ReadableStream.cancel's behavior but handle it internally +- On Node 18, do not combine Response.body's return and AbortController which causes a memory leak diff --git a/package.json b/package.json index 1339610190..55486244f2 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "ci:lint": "eslint --ext .ts . --output-file eslint_report.json --format json", "prettier": "prettier --ignore-path .gitignore --ignore-path .prettierignore --write --list-different .", "prettier:check": "prettier --ignore-path .gitignore --ignore-path .prettierignore --check .", - "test": "jest --detectOpenHandles --detectLeaks --logHeapUsage --runInBand", + "test": "jest --detectOpenHandles --detectLeaks", "prerelease": "yarn build", "prerelease-canary": "yarn build", "release": "changeset publish" diff --git a/packages/fetch/dist/create-node-ponyfill.js b/packages/fetch/dist/create-node-ponyfill.js index ee6b815a7f..9d3b47faa1 100644 --- a/packages/fetch/dist/create-node-ponyfill.js +++ b/packages/fetch/dist/create-node-ponyfill.js @@ -1,4 +1,5 @@ const handleFileRequest = require("./handle-file-request"); +const readableStreamToReadable = require("./readableStreamToReadable"); module.exports = function createNodePonyfill(opts = {}) { @@ -79,49 +80,6 @@ module.exports = function createNodePonyfill(opts = {}) { } } - // ReadableStream doesn't handle aborting properly, so we need to patch it - ponyfills.ReadableStream = class PonyfillReadableStream extends ponyfills.ReadableStream { - constructor(underlyingSource, ...opts) { - super({ - ...underlyingSource, - cancel: (e) => { - this.cancelled = true; - if (underlyingSource.cancel) { - return underlyingSource.cancel(e); - } - } - }, ...opts); - this.underlyingSource = underlyingSource; - } - [Symbol.asyncIterator]() { - const asyncIterator = super[Symbol.asyncIterator](); - return { - next: (...args) => asyncIterator.next(...args), - throw: (...args) => asyncIterator.throw(...args), - return: async (e) => { - const originalResult = await asyncIterator.return(e); - if (!this.cancelled) { - this.cancelled = true; - if (this.underlyingSource.cancel) { - await this.underlyingSource.cancel(e); - } - } - return originalResult; - } - } - } - async cancel(e) { - const originalResult = !super.locked && await super.cancel(e); - if (!this.cancelled) { - this.cancelled = true; - if (this.underlyingSource.cancel) { - await this.underlyingSource.cancel(e); - } - } - return originalResult; - } - } - if (!ponyfills.crypto) { const cryptoModule = require("crypto"); ponyfills.crypto = cryptoModule.webcrypto; @@ -252,7 +210,7 @@ module.exports = function createNodePonyfill(opts = {}) { options.body = streams.Readable.from(encoder.encode()); } if (options.body[Symbol.toStringTag] === 'ReadableStream') { - options.body = streams.Readable.fromWeb ? streams.Readable.fromWeb(options.body) : streams.Readable.from(options.body); + options.body = readableStreamToReadable(options.body); } } super(requestOrUrl, options); @@ -270,7 +228,43 @@ module.exports = function createNodePonyfill(opts = {}) { if (requestOrUrl.url.startsWith('file:')) { return handleFileRequest(requestOrUrl.url, ponyfills.Response); } - return realFetch(requestOrUrl); + const abortCtrl = new ponyfills.AbortController(); + + return realFetch(requestOrUrl, { + ...options, + signal: abortCtrl.signal + }).then(res => { + return new Proxy(res, { + get(target, prop, receiver) { + if (prop === 'body') { + return new Proxy(res.body, { + get(target, prop, receiver) { + if (prop === Symbol.asyncIterator) { + return () => { + const originalAsyncIterator = target[Symbol.asyncIterator](); + return { + next() { + return originalAsyncIterator.next(); + }, + return() { + abortCtrl.abort(); + return originalAsyncIterator.return(); + }, + throw(error) { + abortCtrl.abort(error); + return originalAsyncIterator.throw(error); + } + } + } + } + return Reflect.get(target, prop, receiver); + } + }) + } + return Reflect.get(target, prop, receiver); + } + }) + }); }; ponyfills.fetch = fetch; @@ -278,16 +272,7 @@ module.exports = function createNodePonyfill(opts = {}) { const OriginalResponse = ponyfills.Response || nodeFetch.Response; ponyfills.Response = function Response(body, init) { if (body != null && body[Symbol.toStringTag] === 'ReadableStream') { - const actualBody = streams.Readable.fromWeb ? streams.Readable.fromWeb(body) : streams.Readable.from(body, { - emitClose: true, - autoDestroy: true, - }); - actualBody.on('pause', () => { - body.cancel(); - }) - actualBody.on('close', () => { - body.cancel(); - }) + const actualBody = readableStreamToReadable(body); // Polyfill ReadableStream is not working well with node-fetch's Response return new OriginalResponse(actualBody, init); } diff --git a/packages/fetch/dist/readableStreamToReadable.js b/packages/fetch/dist/readableStreamToReadable.js new file mode 100644 index 0000000000..90c5bcf256 --- /dev/null +++ b/packages/fetch/dist/readableStreamToReadable.js @@ -0,0 +1,19 @@ +const streams = require('stream'); + +module.exports = function readableStreamToReadable(readableStream) { + return streams.Readable.from({ + [Symbol.asyncIterator]() { + const reader = readableStream.getReader(); + return { + next() { + return reader.read(); + }, + async return() { + reader.releaseLock(); + await readableStream.cancel(); + return Promise.resolve({ done: true }); + } + } + } + }); +} \ No newline at end of file diff --git a/packages/server/src/utils.ts b/packages/server/src/utils.ts index ea445f74d0..b72aea1802 100644 --- a/packages/server/src/utils.ts +++ b/packages/server/src/utils.ts @@ -181,31 +181,6 @@ export async function sendNodeResponse( body.destroy(); }); body.pipe(serverResponse); - } else if (isReadableStream(body)) { - const reader = body.getReader(); - serverResponse.once('close', () => { - reader.cancel().finally(() => { - reader.releaseLock(); - body.cancel(); - }); - }); - // eslint-disable-next-line no-inner-declarations - function pump() { - reader - .read() - .then(({ done, value }) => { - if (done) { - serverResponse.end(resolve); - return; - } - serverResponse.write(value, pump); - }) - .catch(error => { - console.error(error); - serverResponse.end(resolve); - }); - } - pump(); } else if (isAsyncIterable(body)) { for await (const chunk of body as AsyncIterable) { if (!serverResponse.write(chunk)) { diff --git a/packages/server/test/adapter.fetch.spec.ts b/packages/server/test/adapter.fetch.spec.ts new file mode 100644 index 0000000000..afd4d8472e --- /dev/null +++ b/packages/server/test/adapter.fetch.spec.ts @@ -0,0 +1,152 @@ +import { createServerAdapter } from '../src'; +import { createTestContainer } from './create-test-container'; + +describe('adapter.fetch', () => { + createTestContainer(({ Request }) => { + // Request as first parameter + it('should accept Request as a first argument', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const request = new Request('http://localhost:8080'); + await adapter(request); + expect(handleRequest).toHaveBeenCalledWith(request, expect.anything()); + }); + it('should accept additional parameters as server context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const request = new Request('http://localhost:8080'); + const additionalCtx = { foo: 'bar' }; + await adapter.fetch(request, additionalCtx); + expect(handleRequest).toHaveBeenCalledWith(request, expect.objectContaining(additionalCtx)); + }); + // URL as first parameter + it('should accept URL as a first argument', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const url = new URL('http://localhost:8080'); + await adapter.fetch(url); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url: url.toString(), + }), + expect.anything() + ); + }); + it('should accept URL without a RequestInit but with an additional context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const url = new URL('http://localhost:8080'); + const additionalCtx = { foo: 'bar' }; + await adapter.fetch(url, additionalCtx); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url: url.toString(), + }), + expect.objectContaining(additionalCtx) + ); + }); + it('should accept URL with a RequestInit', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const url = new URL('http://localhost:8080'); + const init = { + method: 'POST', + }; + await adapter.fetch(url, init); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url: url.toString(), + method: init.method, + }), + expect.anything() + ); + }); + it('should accept URL with a RequestInit and additional parameters as server context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const url = new URL('http://localhost:8080'); + const init = { + method: 'POST', + }; + const additionalCtx = { foo: 'bar' }; + await adapter.fetch(url, init, additionalCtx); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url: url.toString(), + method: init.method, + }), + expect.objectContaining(additionalCtx) + ); + }); + + // String as first parameter + it('should accept string as a first argument', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const url = 'http://localhost:8080/'; + await adapter.fetch(url); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url, + }), + expect.anything() + ); + }); + it('should accept string without a RequestInit but with an additional context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const url = 'http://localhost:8080/'; + const additionalCtx = { foo: 'bar' }; + await adapter.fetch(url, additionalCtx); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url, + }), + expect.objectContaining(additionalCtx) + ); + }); + it('should accept string with a RequestInit', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const url = 'http://localhost:8080/'; + const init = { + method: 'POST', + }; + await adapter.fetch(url, init); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url, + method: init.method, + }), + expect.anything() + ); + }); + it('should accept string with a RequestInit and additional parameters as server context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const url = 'http://localhost:8080/'; + const init = { + method: 'POST', + }; + const additionalCtx = { foo: 'bar' }; + await adapter.fetch(url, init, additionalCtx); + expect(handleRequest).toHaveBeenCalledWith( + expect.objectContaining({ + url, + method: init.method, + }), + expect.objectContaining(additionalCtx) + ); + }); + }); +}); diff --git a/packages/server/test/create-test-container.ts b/packages/server/test/create-test-container.ts new file mode 100644 index 0000000000..21f89e3e11 --- /dev/null +++ b/packages/server/test/create-test-container.ts @@ -0,0 +1,16 @@ +import { createFetch } from '@whatwg-node/fetch'; + +export function createTestContainer( + fn: (fetchAPI: ReturnType) => void, + extraFlags: Parameters[0] = {} +) { + ['default-fetch' /*, 'node-fetch' */].forEach(fetchImplementation => { + describe(fetchImplementation, () => { + const fetchAPI = createFetch({ + useNodeFetch: fetchImplementation === 'node-fetch', + ...extraFlags, + }); + fn(fetchAPI); + }); + }); +} diff --git a/packages/server/test/fetch-event-listener.spec.ts b/packages/server/test/fetch-event-listener.spec.ts index 17eedba416..138d5de961 100644 --- a/packages/server/test/fetch-event-listener.spec.ts +++ b/packages/server/test/fetch-event-listener.spec.ts @@ -1,50 +1,52 @@ import { CustomEvent } from '@whatwg-node/events'; -import { Request, Response } from '@whatwg-node/fetch'; import { createServerAdapter } from '../src'; +import { createTestContainer } from './create-test-container'; describe('FetchEvent listener', () => { - it('should not return a promise to event listener', () => { - const response = new Response(); - const response$ = Promise.resolve(response); - const adapter = createServerAdapter(() => response$); - const respondWith = jest.fn(); - const waitUntil = jest.fn(); - const fetchEvent = Object.assign(new CustomEvent('fetch'), { - request: new Request('http://localhost:8080'), - respondWith, - waitUntil, + createTestContainer(({ Request, Response }) => { + it('should not return a promise to event listener', () => { + const response = new Response(); + const response$ = Promise.resolve(response); + const adapter = createServerAdapter(() => response$, Request); + const respondWith = jest.fn(); + const waitUntil = jest.fn(); + const fetchEvent = Object.assign(new CustomEvent('fetch'), { + request: new Request('http://localhost:8080'), + respondWith, + waitUntil, + }); + const returnValue = adapter(fetchEvent); + expect(returnValue).toBeUndefined(); + expect(respondWith).toHaveBeenCalledWith(response$); }); - const returnValue = adapter(fetchEvent); - expect(returnValue).toBeUndefined(); - expect(respondWith).toHaveBeenCalledWith(response$); - }); - it('should expose FetchEvent as server context', () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const respondWith = jest.fn(); - const waitUntil = jest.fn(); - const fetchEvent = Object.assign(new CustomEvent('fetch'), { - request: new Request('http://localhost:8080'), - respondWith, - waitUntil, + it('should expose FetchEvent as server context', () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const respondWith = jest.fn(); + const waitUntil = jest.fn(); + const fetchEvent = Object.assign(new CustomEvent('fetch'), { + request: new Request('http://localhost:8080'), + respondWith, + waitUntil, + }); + adapter(fetchEvent); + expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, fetchEvent); }); - adapter(fetchEvent); - expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, fetchEvent); - }); - it('should accept additional parameters as server context', () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const respondWith = jest.fn(); - const waitUntil = jest.fn(); - const fetchEvent = Object.assign(new CustomEvent('fetch'), { - request: new Request('http://localhost:8080'), - respondWith, - waitUntil, + it('should accept additional parameters as server context', () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const respondWith = jest.fn(); + const waitUntil = jest.fn(); + const fetchEvent = Object.assign(new CustomEvent('fetch'), { + request: new Request('http://localhost:8080'), + respondWith, + waitUntil, + }); + const additionalCtx = { foo: 'bar' }; + adapter(fetchEvent, additionalCtx); + expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, expect.objectContaining(additionalCtx)); }); - const additionalCtx = { foo: 'bar' }; - adapter(fetchEvent, additionalCtx); - expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, expect.objectContaining(additionalCtx)); }); }); diff --git a/packages/server/test/fetch.spec.ts b/packages/server/test/fetch.spec.ts deleted file mode 100644 index 5161faf663..0000000000 --- a/packages/server/test/fetch.spec.ts +++ /dev/null @@ -1,150 +0,0 @@ -import { createServerAdapter } from '../src'; -import { Request } from '@whatwg-node/fetch'; - -describe('fetch', () => { - // Request as first parameter - it('should accept Request as a first argument', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const request = new Request('http://localhost:8080'); - await adapter(request); - expect(handleRequest).toHaveBeenCalledWith(request, expect.anything()); - }); - it('should accept additional parameters as server context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const request = new Request('http://localhost:8080'); - const additionalCtx = { foo: 'bar' }; - await adapter.fetch(request, additionalCtx); - expect(handleRequest).toHaveBeenCalledWith(request, expect.objectContaining(additionalCtx)); - }); - // URL as first parameter - it('should accept URL as a first argument', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const url = new URL('http://localhost:8080'); - await adapter.fetch(url); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url: url.toString(), - }), - expect.anything() - ); - }); - it('should accept URL without a RequestInit but with an additional context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const url = new URL('http://localhost:8080'); - const additionalCtx = { foo: 'bar' }; - await adapter.fetch(url, additionalCtx); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url: url.toString(), - }), - expect.objectContaining(additionalCtx) - ); - }); - it('should accept URL with a RequestInit', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const url = new URL('http://localhost:8080'); - const init = { - method: 'POST', - }; - await adapter.fetch(url, init); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url: url.toString(), - method: init.method, - }), - expect.anything() - ); - }); - it('should accept URL with a RequestInit and additional parameters as server context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const url = new URL('http://localhost:8080'); - const init = { - method: 'POST', - }; - const additionalCtx = { foo: 'bar' }; - await adapter.fetch(url, init, additionalCtx); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url: url.toString(), - method: init.method, - }), - expect.objectContaining(additionalCtx) - ); - }); - - // String as first parameter - it('should accept string as a first argument', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const url = 'http://localhost:8080/'; - await adapter.fetch(url); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url, - }), - expect.anything() - ); - }); - it('should accept string without a RequestInit but with an additional context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const url = 'http://localhost:8080/'; - const additionalCtx = { foo: 'bar' }; - await adapter.fetch(url, additionalCtx); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url, - }), - expect.objectContaining(additionalCtx) - ); - }); - it('should accept string with a RequestInit', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const url = 'http://localhost:8080/'; - const init = { - method: 'POST', - }; - await adapter.fetch(url, init); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url, - method: init.method, - }), - expect.anything() - ); - }); - it('should accept string with a RequestInit and additional parameters as server context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const url = 'http://localhost:8080/'; - const init = { - method: 'POST', - }; - const additionalCtx = { foo: 'bar' }; - await adapter.fetch(url, init, additionalCtx); - expect(handleRequest).toHaveBeenCalledWith( - expect.objectContaining({ - url, - method: init.method, - }), - expect.objectContaining(additionalCtx) - ); - }); -}); diff --git a/packages/server/test/formdata.spec.ts b/packages/server/test/formdata.spec.ts index b070ddc256..536914cddd 100644 --- a/packages/server/test/formdata.spec.ts +++ b/packages/server/test/formdata.spec.ts @@ -1,64 +1,63 @@ -import { createFetch } from '@whatwg-node/fetch'; import { createServerAdapter } from '@whatwg-node/server'; -import { createServer, Server } from 'http'; +import { createTestContainer } from './create-test-container'; +import { createTestServer, TestServer } from './test-server'; describe('FormData', () => { - let server: Server; - let url: string; - afterEach(done => { - server.close(done); + let testServer: TestServer; + beforeAll(async () => { + testServer = await createTestServer(); + }); + afterAll(done => { + testServer.server.close(done); }); ['fieldsFirst:true', 'fieldsFirst:false'].forEach(fieldsFirstFlag => { - const fetchAPI = createFetch({ - formDataLimits: { - fieldsFirst: fieldsFirstFlag === 'fieldsFirst:true', - }, - }); - - describe(fieldsFirstFlag, () => { - it('should forward formdata correctly', async () => { - let receivedFieldContent: string | undefined; - let receivedFileName: string | undefined; - let receivedFileType: string | undefined; - let receivedFileContent: string | undefined; - const adapter = createServerAdapter(async request => { - try { - const body = await request.formData(); - receivedFieldContent = body.get('foo') as string; - const file = body.get('baz') as File; - receivedFileName = file.name; - receivedFileType = file.type; - receivedFileContent = await file.text(); - } catch (e: any) { - return new fetchAPI.Response(e.stack, { - status: 500, + createTestContainer( + fetchAPI => { + describe(fieldsFirstFlag, () => { + it('should forward formdata correctly', async () => { + let receivedFieldContent: string | undefined; + let receivedFileName: string | undefined; + let receivedFileType: string | undefined; + let receivedFileContent: string | undefined; + const adapter = createServerAdapter(async request => { + try { + const body = await request.formData(); + receivedFieldContent = body.get('foo') as string; + const file = body.get('baz') as File; + receivedFileName = file.name; + receivedFileType = file.type; + receivedFileContent = await file.text(); + } catch (e: any) { + return new fetchAPI.Response(e.stack, { + status: 500, + }); + } + return new fetchAPI.Response(null, { + status: 204, + }); + }, fetchAPI.Request); + testServer.server.once('request', adapter); + const formData = new fetchAPI.FormData(); + formData.append('foo', 'bar'); + formData.append('baz', new fetchAPI.File(['baz'], 'baz.txt', { type: 'text/plain' })); + const response = await fetchAPI.fetch(testServer.url, { + method: 'POST', + body: formData, }); - } - return new fetchAPI.Response(null, { - status: 204, + expect(await response.text()).toBe(''); + expect(response.status).toBe(204); + expect(receivedFieldContent).toBe('bar'); + expect(receivedFileName).toBe('baz.txt'); + expect(receivedFileType).toBe('text/plain'); + expect(receivedFileContent).toBe('baz'); }); - }, fetchAPI.Request); - server = createServer(adapter); - await new Promise(resolve => { - server.listen(0, () => { - url = `http://localhost:${(server.address() as any).port}`; - resolve(); - }); - }); - const formData = new fetchAPI.FormData(); - formData.append('foo', 'bar'); - formData.append('baz', new fetchAPI.File(['baz'], 'baz.txt', { type: 'text/plain' })); - const response = await fetchAPI.fetch(url, { - method: 'POST', - body: formData, }); - expect(await response.text()).toBe(''); - expect(response.status).toBe(204); - expect(receivedFieldContent).toBe('bar'); - expect(receivedFileName).toBe('baz.txt'); - expect(receivedFileType).toBe('text/plain'); - expect(receivedFileContent).toBe('baz'); - }); - }); + }, + { + formDataLimits: { + fieldsFirst: fieldsFirstFlag === 'fieldsFirst:true', + }, + } + ); }); }); diff --git a/packages/server/test/node.spec.ts b/packages/server/test/node.spec.ts index 3c3ecc84d8..272a0bfc9e 100644 --- a/packages/server/test/node.spec.ts +++ b/packages/server/test/node.spec.ts @@ -1,28 +1,24 @@ import { createServerAdapter } from '@whatwg-node/server'; -import { fetch, Response, ReadableStream, AbortController } from '@whatwg-node/fetch'; -import { createServer, IncomingMessage, Server, ServerResponse } from 'http'; -import { AddressInfo } from 'net'; +import { fetch, Response, ReadableStream } from '@whatwg-node/fetch'; +import { IncomingMessage, ServerResponse } from 'http'; +import { createTestServer, TestServer } from './test-server'; describe('Node Specific Cases', () => { - let server: Server; - let url: string; - beforeEach(done => { - server = createServer(); - server.listen(0, () => { - url = `http://localhost:${(server.address() as AddressInfo).port}`; - done(); - }); + let testServer: TestServer; + beforeAll(async () => { + testServer = await createTestServer(); }); - afterEach(done => { - server.close(done); + afterAll(done => { + testServer.server.close(done); }); + it('should handle empty responses', async () => { const serverAdapter = createServerAdapter(() => { return undefined as any; }); - server.once('request', serverAdapter); - const response = await fetch(url); + testServer.server.once('request', serverAdapter); + const response = await fetch(testServer.url); await response.text(); expect(response.status).toBe(404); }); @@ -39,8 +35,8 @@ describe('Node Specific Cases', () => { status: 204, }); }); - server.once('request', serverAdapter); - const response$ = fetch(url); + testServer.server.once('request', serverAdapter); + const response$ = fetch(testServer.url); const response = await response$; await response.text(); expect(flag).toBe(false); @@ -60,8 +56,8 @@ describe('Node Specific Cases', () => { foo: string; }>(handleRequest); const additionalCtx = { foo: 'bar' }; - server.once('request', (...args) => serverAdapter(...args, additionalCtx)); - const response = await fetch(url); + testServer.server.once('request', (...args) => serverAdapter(...args, additionalCtx)); + const response = await fetch(testServer.url); await response.text(); expect(handleRequest).toHaveBeenCalledWith(expect.anything(), expect.objectContaining(additionalCtx)); }); @@ -80,11 +76,9 @@ describe('Node Specific Cases', () => { }) ) ); - server.once('request', serverAdapter); - const abortCtrl = new AbortController(); - const response = await fetch(url, { - signal: abortCtrl.signal, - }); + + testServer.server.once('request', serverAdapter); + const response = await fetch(testServer.url); const collectedValues: string[] = []; @@ -97,11 +91,9 @@ describe('Node Specific Cases', () => { i++; } - abortCtrl.abort(); - expect(collectedValues).toHaveLength(3); await sleep(100); - expect(cancelFn).toHaveBeenCalled(); + expect(cancelFn).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/server/test/request-container.spec.ts b/packages/server/test/request-container.spec.ts index 4cefdfa292..9dda33aa13 100644 --- a/packages/server/test/request-container.spec.ts +++ b/packages/server/test/request-container.spec.ts @@ -1,26 +1,28 @@ -import { Request } from '@whatwg-node/fetch'; import { createServerAdapter } from '../src'; +import { createTestContainer } from './create-test-container'; describe('Request Container', () => { - it('should receive correct request and container as a context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter(handleRequest); - const requestContainer = { - request: new Request('http://localhost:8080'), - }; - await adapter(requestContainer); - expect(handleRequest).toHaveBeenCalledWith(requestContainer.request, expect.objectContaining(requestContainer)); - }); - it('should accept additional parameters as server context', async () => { - const handleRequest = jest.fn(); - const adapter = createServerAdapter<{ - foo: string; - }>(handleRequest); - const requestContainer = { - request: new Request('http://localhost:8080'), - foo: 'bar', - }; - await adapter(requestContainer); - expect(handleRequest).toHaveBeenCalledWith(requestContainer.request, expect.objectContaining(requestContainer)); + createTestContainer(({ Request }) => { + it('should receive correct request and container as a context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter(handleRequest, Request); + const requestContainer = { + request: new Request('http://localhost:8080'), + }; + await adapter(requestContainer); + expect(handleRequest).toHaveBeenCalledWith(requestContainer.request, expect.objectContaining(requestContainer)); + }); + it('should accept additional parameters as server context', async () => { + const handleRequest = jest.fn(); + const adapter = createServerAdapter<{ + foo: string; + }>(handleRequest, Request); + const requestContainer = { + request: new Request('http://localhost:8080'), + foo: 'bar', + }; + await adapter(requestContainer); + expect(handleRequest).toHaveBeenCalledWith(requestContainer.request, expect.objectContaining(requestContainer)); + }); }); }); diff --git a/packages/server/test/request-listener.spec.ts b/packages/server/test/request-listener.spec.ts index 9b1e56620c..d0464d9e17 100644 --- a/packages/server/test/request-listener.spec.ts +++ b/packages/server/test/request-listener.spec.ts @@ -1,8 +1,7 @@ import { createServerAdapter } from '@whatwg-node/server'; -import { createFetch } from '@whatwg-node/fetch'; import { Readable } from 'stream'; -import { createServer, Server } from 'http'; -import { AddressInfo } from 'net'; +import { createTestServer, TestServer } from './test-server'; +import { createTestContainer } from './create-test-container'; const methodsWithoutBody = ['GET', 'DELETE']; @@ -37,41 +36,36 @@ async function compareResponse(toBeChecked: Response, expected: Response) { }); } -let server: Server; -let url: string; - describe('Request Listener', () => { - afterEach(done => { - server?.close(done); + let testServer: TestServer; + beforeAll(async () => { + testServer = await createTestServer(); }); - // TODO: add node-fetch here - ['default-fetch'].forEach(fetchImplementation => { - describe(fetchImplementation, () => { - const fetchAPI = createFetch({ - useNodeFetch: fetchImplementation === 'node-fetch', - }); + afterAll(done => { + testServer.server.close(done); + }); - async function compareReadableStream(toBeChecked: ReadableStream | null, expected: BodyInit | null) { - if (expected != null) { - expect(toBeChecked).toBeTruthy(); - const expectedBody = new fetchAPI.Response(expected).body; - const expectedStream = Readable.from(expectedBody as any); - const expectedIterator = expectedStream[Symbol.asyncIterator](); - const toBeCheckedStream = Readable.from(toBeChecked as any); - for await (const toBeCheckedChunk of toBeCheckedStream) { - if (toBeCheckedChunk) { - const toBeCheckedValues = Buffer.from(toBeCheckedChunk).toString().trim().split('\n'); - for (const toBeCheckedValue of toBeCheckedValues) { - const trimmedToBeCheckedValue = toBeCheckedValue.trim(); - if (trimmedToBeCheckedValue) { - const expectedResult = await expectedIterator.next(); - const expectedChunk = expectedResult.value; - if (expectedChunk) { - const expectedValue = Buffer.from(expectedResult.value).toString().trim(); - if (expectedValue) { - expect(trimmedToBeCheckedValue).toBe(expectedValue); - } + // TODO: add node-fetch here + createTestContainer(fetchAPI => { + async function compareReadableStream(toBeCheckedStream: ReadableStream | null, expected: BodyInit | null) { + if (expected != null) { + expect(toBeCheckedStream).toBeTruthy(); + const expectedStream = + typeof expected === 'object' && Symbol.asyncIterator in expected ? expected : Readable.from(expected as any); + const expectedIterator = expectedStream[Symbol.asyncIterator](); + for await (const toBeCheckedChunk of toBeCheckedStream as any as AsyncIterable) { + if (toBeCheckedChunk) { + const toBeCheckedValues = Buffer.from(toBeCheckedChunk).toString().trim().split('\n'); + for (const toBeCheckedValue of toBeCheckedValues) { + const trimmedToBeCheckedValue = toBeCheckedValue.trim(); + if (trimmedToBeCheckedValue) { + const expectedResult = await expectedIterator.next(); + const expectedChunk = expectedResult.value; + if (expectedChunk) { + const expectedValue = Buffer.from(expectedResult.value).toString().trim(); + if (expectedValue) { + expect(trimmedToBeCheckedValue).toBe(expectedValue); } } } @@ -79,148 +73,142 @@ describe('Request Listener', () => { } } } - - async function runTestForRequestAndResponse({ - requestInit, - expectedResponse, - getRequestBody, - getResponseBody, - }: { - requestInit: RequestInit; - expectedResponse: Response; - getRequestBody: () => BodyInit; - getResponseBody: () => BodyInit; - }) { - const adapter = createServerAdapter(async (request: Request) => { - await compareRequest(request, expectedRequest); - if (methodsWithBody.includes(expectedRequest.method)) { - await compareReadableStream(request.body, getRequestBody()); + } + + async function runTestForRequestAndResponse({ + requestInit, + expectedResponse, + getRequestBody, + getResponseBody, + }: { + requestInit: RequestInit; + expectedResponse: Response; + getRequestBody: () => BodyInit; + getResponseBody: () => BodyInit; + }) { + const adapter = createServerAdapter(async (request: Request) => { + await compareRequest(request, expectedRequest); + if (methodsWithBody.includes(expectedRequest.method)) { + await compareReadableStream(request.body, getRequestBody()); + } + return expectedResponse; + }, fetchAPI.Request); + testServer.server.once('request', adapter); + const expectedRequest = new fetchAPI.Request(testServer.url, requestInit); + const returnedResponse = await fetchAPI.fetch(expectedRequest); + await compareResponse(returnedResponse, expectedResponse); + await compareReadableStream(returnedResponse.body, getResponseBody()); + } + + function getRegularRequestBody() { + return JSON.stringify({ requestFoo: 'requestFoo' }); + } + + function getRegularResponseBody() { + return JSON.stringify({ responseFoo: 'responseFoo' }); + } + + function getIncrementalRequestBody() { + return new fetchAPI.ReadableStream({ + async start(controller) { + for (let i = 0; i < 2; i++) { + await new Promise(resolve => setTimeout(resolve, 30)); + controller.enqueue(`data: request_${i.toString()}\n`); } - return expectedResponse; - }, fetchAPI.Request); - server = createServer(adapter); - await new Promise(resolve => { - server.listen(0, () => { - url = `http://localhost:${(server.address() as AddressInfo).port}`; - resolve(); - }); - }); - const expectedRequest = new fetchAPI.Request(url, requestInit); - const returnedResponse = await fetchAPI.fetch(expectedRequest); - await compareResponse(returnedResponse, expectedResponse); - await compareReadableStream(returnedResponse.body, getResponseBody()); - } - - function getRegularRequestBody() { - return JSON.stringify({ requestFoo: 'requestFoo' }); - } - - function getRegularResponseBody() { - return JSON.stringify({ responseFoo: 'responseFoo' }); - } - - function getIncrementalRequestBody() { - return new fetchAPI.ReadableStream({ - async start(controller) { - for (let i = 0; i < 2; i++) { - await new Promise(resolve => setTimeout(resolve, 30)); - controller.enqueue(`data: request_${i.toString()}\n`); - } - controller.close(); + controller.close(); + }, + }); + } + + function getIncrementalResponseBody() { + return new fetchAPI.ReadableStream({ + async start(controller) { + for (let i = 0; i < 10; i++) { + await new Promise(resolve => setTimeout(resolve, 30)); + controller.enqueue(`data: response_${i.toString()}\n`); + } + controller.close(); + }, + }); + } + + [...methodsWithBody, ...methodsWithoutBody].forEach(method => { + it(`should handle regular requests with ${method}`, async () => { + const requestInit: RequestInit = { + method, + headers: { + accept: 'application/json', + 'content-type': 'application/json', + 'random-header': Date.now().toString(), + }, + }; + if (methodsWithBody.includes(method)) { + requestInit.body = getRegularRequestBody(); + } + const expectedResponse = new fetchAPI.Response(getRegularResponseBody(), { + status: 200, + headers: { + 'content-type': 'application/json', + 'random-header': Date.now().toString(), }, }); - } + await runTestForRequestAndResponse({ + requestInit, + getRequestBody: getRegularRequestBody, + expectedResponse, + getResponseBody: getRegularResponseBody, + }); + }); - function getIncrementalResponseBody() { - return new fetchAPI.ReadableStream({ - async start(controller) { - for (let i = 0; i < 10; i++) { - await new Promise(resolve => setTimeout(resolve, 30)); - controller.enqueue(`data: response_${i.toString()}\n`); - } - controller.close(); + it(`should handle incremental responses with ${method}`, async () => { + const requestInit: RequestInit = { + method, + headers: { + accept: 'application/json', + 'random-header': Date.now().toString(), + }, + }; + if (methodsWithBody.includes(method)) { + requestInit.body = getRegularRequestBody(); + } + const expectedResponse = new fetchAPI.Response(getIncrementalResponseBody(), { + status: 200, + headers: { + 'content-type': 'application/json', + 'random-header': Date.now().toString(), }, }); - } - - [...methodsWithBody, ...methodsWithoutBody].forEach(method => { - it(`should handle regular requests with ${method}`, async () => { - const requestInit: RequestInit = { - method, - headers: { - accept: 'application/json', - 'content-type': 'application/json', - 'random-header': Date.now().toString(), - }, - }; - if (methodsWithBody.includes(method)) { - requestInit.body = getRegularRequestBody(); - } - const expectedResponse = new fetchAPI.Response(getRegularResponseBody(), { - status: 200, - headers: { - 'content-type': 'application/json', - 'random-header': Date.now().toString(), - }, - }); - await runTestForRequestAndResponse({ - requestInit, - getRequestBody: getRegularRequestBody, - expectedResponse, - getResponseBody: getRegularResponseBody, - }); + await runTestForRequestAndResponse({ + requestInit, + getRequestBody: getRegularRequestBody, + expectedResponse, + getResponseBody: getIncrementalResponseBody, }); + }); - it(`should handle incremental responses with ${method}`, async () => { - const requestInit: RequestInit = { - method, - headers: { - accept: 'application/json', - 'random-header': Date.now().toString(), - }, - }; - if (methodsWithBody.includes(method)) { - requestInit.body = getRegularRequestBody(); - } - const expectedResponse = new fetchAPI.Response(getIncrementalResponseBody(), { - status: 200, - headers: { - 'content-type': 'application/json', - 'random-header': Date.now().toString(), - }, - }); - await runTestForRequestAndResponse({ - requestInit, - getRequestBody: getRegularRequestBody, - expectedResponse, - getResponseBody: getIncrementalResponseBody, - }); + it(`should handle incremental requests with ${method}`, async () => { + const requestInit: RequestInit = { + method, + headers: { + accept: 'application/json', + 'random-header': Date.now().toString(), + }, + }; + if (methodsWithBody.includes(method)) { + requestInit.body = getIncrementalRequestBody(); + } + const expectedResponse = new fetchAPI.Response(getRegularResponseBody(), { + status: 200, + headers: { + 'content-type': 'application/json', + 'random-header': Date.now().toString(), + }, }); - - it(`should handle incremental requests with ${method}`, async () => { - const requestInit: RequestInit = { - method, - headers: { - accept: 'application/json', - 'random-header': Date.now().toString(), - }, - }; - if (methodsWithBody.includes(method)) { - requestInit.body = getIncrementalRequestBody(); - } - const expectedResponse = new fetchAPI.Response(getRegularResponseBody(), { - status: 200, - headers: { - 'content-type': 'application/json', - 'random-header': Date.now().toString(), - }, - }); - await runTestForRequestAndResponse({ - requestInit, - getRequestBody: getIncrementalRequestBody, - expectedResponse, - getResponseBody: getRegularResponseBody, - }); + await runTestForRequestAndResponse({ + requestInit, + getRequestBody: getIncrementalRequestBody, + expectedResponse, + getResponseBody: getRegularResponseBody, }); }); }); diff --git a/packages/server/test/test-server.ts b/packages/server/test/test-server.ts new file mode 100644 index 0000000000..199f139786 --- /dev/null +++ b/packages/server/test/test-server.ts @@ -0,0 +1,18 @@ +import { createServer, Server } from 'http'; +import { AddressInfo } from 'net'; + +export interface TestServer { + url: string; + server: Server; +} + +export function createTestServer(): Promise { + const server = createServer(); + return new Promise(resolve => { + server.listen(0, () => { + const addressInfo = server.address() as AddressInfo; + const url = `http://localhost:${addressInfo.port}`; + resolve({ server, url }); + }); + }); +}