From 9490fd2f1d5e1bd8f9a233e3c2e6ede69b0c6e8f Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Thu, 14 Dec 2023 13:06:19 +0800 Subject: [PATCH 01/14] feat: support redirections --- sources/httpUtils.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sources/httpUtils.ts b/sources/httpUtils.ts index 67cf98910..384ccd849 100644 --- a/sources/httpUtils.ts +++ b/sources/httpUtils.ts @@ -13,14 +13,20 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) const proxyAgent = new ProxyAgent(); return new Promise((resolve, reject) => { - const request = https.get(url, {...options, agent: proxyAgent}, response => { + const creatRequest = (url: string) => https.get(url, {...options, agent: proxyAgent}, response => { const statusCode = response.statusCode; + + if (statusCode === 301 || statusCode === 302) + return creatRequest(response.headers.location); + if (statusCode != null && statusCode >= 200 && statusCode < 300) return resolve(response); return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); }); + const request = creatRequest(url); + request.on(`error`, err => { reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); }); From 5d593a9d791b996447ddcd71030eb557b7e41f32 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Thu, 14 Dec 2023 13:32:43 +0800 Subject: [PATCH 02/14] feat 307 308 supported --- sources/httpUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/httpUtils.ts b/sources/httpUtils.ts index 384ccd849..8bc0dee8d 100644 --- a/sources/httpUtils.ts +++ b/sources/httpUtils.ts @@ -16,7 +16,7 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) const creatRequest = (url: string) => https.get(url, {...options, agent: proxyAgent}, response => { const statusCode = response.statusCode; - if (statusCode === 301 || statusCode === 302) + if ([301, 302, 307, 308].includes(statusCode)) return creatRequest(response.headers.location); if (statusCode != null && statusCode >= 200 && statusCode < 300) From 33d99dcd983b0d49f81725dfed8be349cfbc03a1 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Thu, 14 Dec 2023 17:27:00 +0800 Subject: [PATCH 03/14] fix spell wrong --- sources/httpUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sources/httpUtils.ts b/sources/httpUtils.ts index 8bc0dee8d..aea86791e 100644 --- a/sources/httpUtils.ts +++ b/sources/httpUtils.ts @@ -13,11 +13,11 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) const proxyAgent = new ProxyAgent(); return new Promise((resolve, reject) => { - const creatRequest = (url: string) => https.get(url, {...options, agent: proxyAgent}, response => { + const createRequest = (url: string) => https.get(url, {...options, agent: proxyAgent}, response => { const statusCode = response.statusCode; if ([301, 302, 307, 308].includes(statusCode)) - return creatRequest(response.headers.location); + return createRequest(response.headers.location); if (statusCode != null && statusCode >= 200 && statusCode < 300) return resolve(response); @@ -25,7 +25,7 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); }); - const request = creatRequest(url); + const request = createRequest(url); request.on(`error`, err => { reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); From 02522cb9c221932f6364033ae1c284e3155bfe55 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sat, 16 Dec 2023 14:18:44 +0800 Subject: [PATCH 04/14] feat: handle `error` for redirection --- sources/httpUtils.ts | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/sources/httpUtils.ts b/sources/httpUtils.ts index aea86791e..375dd5a8f 100644 --- a/sources/httpUtils.ts +++ b/sources/httpUtils.ts @@ -1,6 +1,6 @@ -import {UsageError} from 'clipanion'; -import {RequestOptions} from 'https'; -import {IncomingMessage} from 'http'; +import {UsageError} from 'clipanion'; +import {RequestOptions} from 'https'; +import {IncomingMessage, ClientRequest} from 'http'; export async function fetchUrlStream(url: string, options: RequestOptions = {}) { if (process.env.COREPACK_ENABLE_NETWORK === `0`) @@ -13,23 +13,25 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) const proxyAgent = new ProxyAgent(); return new Promise((resolve, reject) => { - const createRequest = (url: string) => https.get(url, {...options, agent: proxyAgent}, response => { - const statusCode = response.statusCode; + const createRequest = (url: string) => { + const request: ClientRequest = https.get(url, {...options, agent: proxyAgent}, response => { + const statusCode = response.statusCode; - if ([301, 302, 307, 308].includes(statusCode)) - return createRequest(response.headers.location); + if ([301, 302, 307, 308].includes(statusCode as number)) + return createRequest(response.headers.location as string); - if (statusCode != null && statusCode >= 200 && statusCode < 300) - return resolve(response); + if (statusCode != null && statusCode >= 200 && statusCode < 300) + return resolve(response); - return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); - }); + return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); + }); - const request = createRequest(url); + request.on(`error`, err => { + reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); + }); + }; - request.on(`error`, err => { - reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); - }); + createRequest(url); }); } From 1a58ba0870670a3a9175468072b20758d404a9b6 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sat, 16 Dec 2023 14:19:06 +0800 Subject: [PATCH 05/14] feat: add httpUtils tests --- tests/httpUtils.test.ts | 91 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 tests/httpUtils.test.ts diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts new file mode 100644 index 000000000..f200caa5a --- /dev/null +++ b/tests/httpUtils.test.ts @@ -0,0 +1,91 @@ +import {jest, describe, beforeEach, beforeAll, it, expect} from '@jest/globals'; + +import {fetchUrlStream} from '../sources/httpUtils'; + + +describe(`http utils fetchUrlStream`, () => { + const getUrl = (statusCode: number | string, redirectCode?: number | string) => + `https://registry.example.org/answered/${statusCode}${redirectCode ? `?redirectCode=${redirectCode}` : ``}`; + + const httpsGetFn = jest.fn((url: string, _, callback: (response: any) => void) => { + const parsedUrl = new URL(url); + const {1: statusCode} = parsedUrl.pathname.match(/\/(\d+|error)/) as Array; + const response = {url, statusCode: +statusCode}; + const errorCallbacks: Array<(err: string) => void> = []; + + if ([301, 302, 307, 308].includes(+statusCode)) { + const redirectCode = parsedUrl.searchParams.get(`redirectCode`) as string; + // mock response.headers.location + Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); + } + + // handle request.on('error', err => ...) + if (statusCode === `error`) + process.nextTick(() => errorCallbacks.forEach(cb => cb(`Test internal error`))); + else + callback(response); + + return { + on: (type: string, callback: (err: string) => void) => { + if (type === `error`) { + errorCallbacks.push(callback); + } + }, + }; + }); + + beforeAll(() => { + jest.doMock(`https`, () => ({ + get: httpsGetFn, + Agent: class Agent {}, + })); + }); + + beforeEach(() => { + httpsGetFn.mockClear(); + }); + + it(`correct response answered statusCode should be >= 200 and < 300`, async () => { + await expect(fetchUrlStream(getUrl(200))).resolves.toMatchObject({ + statusCode: 200, + }); + + await expect(fetchUrlStream(getUrl(299))).resolves.toMatchObject({ + statusCode: 299, + }); + + expect(httpsGetFn).toHaveBeenCalledTimes(2); + }); + + it(`bed response`, async () => { + await expect(fetchUrlStream(getUrl(300))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(199))).rejects.toThrowError(); + }); + + it(`redirection with correct response`, async () => { + await expect(fetchUrlStream(getUrl(301, 200))).resolves.toMatchObject({ + statusCode: 200, + }); + + expect(httpsGetFn).toHaveBeenCalledTimes(2); + + await expect(fetchUrlStream(getUrl(308, 299))).resolves.toMatchObject({ + statusCode: 299, + }); + + expect(httpsGetFn).toHaveBeenCalledTimes(4); + }); + + it(`redirection with bed response`, async () => { + await expect(fetchUrlStream(getUrl(301, 300))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(308, 199))).rejects.toThrowError(); + }); + + it(`rejects with error`, async () => { + await expect(fetchUrlStream(getUrl(`error`))).rejects.toThrowError(); + }); + + it(`rejects when redirection with error`, async () => { + await expect(fetchUrlStream(getUrl(307, `error`))).rejects.toThrowError(); + }); +}); From eef8cdd0c7078fc25b9d3d75d3df09c7623b2448 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sun, 17 Dec 2023 21:01:54 +0800 Subject: [PATCH 06/14] Update tests/httpUtils.test.ts Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index f200caa5a..56a96e0c5 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -57,7 +57,7 @@ describe(`http utils fetchUrlStream`, () => { expect(httpsGetFn).toHaveBeenCalledTimes(2); }); - it(`bed response`, async () => { + it(`bad response`, async () => { await expect(fetchUrlStream(getUrl(300))).rejects.toThrowError(); await expect(fetchUrlStream(getUrl(199))).rejects.toThrowError(); }); From 69d4eea58e8eafc98b337e3be799eb26ca555810 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sun, 17 Dec 2023 21:05:00 +0800 Subject: [PATCH 07/14] fix: wrong word Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index 56a96e0c5..e623916df 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -76,7 +76,7 @@ describe(`http utils fetchUrlStream`, () => { expect(httpsGetFn).toHaveBeenCalledTimes(4); }); - it(`redirection with bed response`, async () => { + it(`redirection with bad response`, async () => { await expect(fetchUrlStream(getUrl(301, 300))).rejects.toThrowError(); await expect(fetchUrlStream(getUrl(308, 199))).rejects.toThrowError(); }); From 41fc612f9c4138fa5f6434250d9f85a9e6f20e38 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sun, 17 Dec 2023 21:08:02 +0800 Subject: [PATCH 08/14] fix: misnamed Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index e623916df..520020c7a 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -8,7 +8,7 @@ describe(`http utils fetchUrlStream`, () => { `https://registry.example.org/answered/${statusCode}${redirectCode ? `?redirectCode=${redirectCode}` : ``}`; const httpsGetFn = jest.fn((url: string, _, callback: (response: any) => void) => { - const parsedUrl = new URL(url); + const parsedURL = new URL(url); const {1: statusCode} = parsedUrl.pathname.match(/\/(\d+|error)/) as Array; const response = {url, statusCode: +statusCode}; const errorCallbacks: Array<(err: string) => void> = []; From 7101e288937ba36b7cf72320edd80f6cac8f5180 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sun, 17 Dec 2023 21:19:56 +0800 Subject: [PATCH 09/14] fix: misnamed and definition is not concis Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index 520020c7a..c3fc1caa9 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -14,7 +14,7 @@ describe(`http utils fetchUrlStream`, () => { const errorCallbacks: Array<(err: string) => void> = []; if ([301, 302, 307, 308].includes(+statusCode)) { - const redirectCode = parsedUrl.searchParams.get(`redirectCode`) as string; + const redirectCode = parsedURL.searchParams.get(`redirectCode`)!; // mock response.headers.location Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); } From 61645309b063dd631982b711bf379e072374b824 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Sun, 17 Dec 2023 21:26:45 +0800 Subject: [PATCH 10/14] fix: better way to get statusCode Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index c3fc1caa9..c14bb0f5f 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -9,7 +9,7 @@ describe(`http utils fetchUrlStream`, () => { const httpsGetFn = jest.fn((url: string, _, callback: (response: any) => void) => { const parsedURL = new URL(url); - const {1: statusCode} = parsedUrl.pathname.match(/\/(\d+|error)/) as Array; + const statusCode = parsedURL.pathname.slice(parsedURL.pathname.lastIndexOf(`/`) + 1); const response = {url, statusCode: +statusCode}; const errorCallbacks: Array<(err: string) => void> = []; From 2be83933aa5c80cded2cae34279846808e6f99e4 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Wed, 27 Dec 2023 09:52:53 +0800 Subject: [PATCH 11/14] Update sources/httpUtils.ts Co-authored-by: Antoine du Hamel --- sources/httpUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/httpUtils.ts b/sources/httpUtils.ts index 375dd5a8f..94598dfbe 100644 --- a/sources/httpUtils.ts +++ b/sources/httpUtils.ts @@ -17,7 +17,7 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) const request: ClientRequest = https.get(url, {...options, agent: proxyAgent}, response => { const statusCode = response.statusCode; - if ([301, 302, 307, 308].includes(statusCode as number)) + if ([301, 302, 307, 308].includes(statusCode as number) && response.headers.location) return createRequest(response.headers.location as string); if (statusCode != null && statusCode >= 200 && statusCode < 300) From 5caead764a2362848be299bb9e5bf45abdffd410 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Wed, 27 Dec 2023 23:16:14 +0800 Subject: [PATCH 12/14] Update tests/httpUtils.test.ts Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index c14bb0f5f..12b6dcd0b 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -16,7 +16,7 @@ describe(`http utils fetchUrlStream`, () => { if ([301, 302, 307, 308].includes(+statusCode)) { const redirectCode = parsedURL.searchParams.get(`redirectCode`)!; // mock response.headers.location - Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); + if (redirectCode) Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); } // handle request.on('error', err => ...) From dda29e2ef0755d5bdc0f24ed87a371cc2260851f Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Wed, 27 Dec 2023 23:16:30 +0800 Subject: [PATCH 13/14] Update tests/httpUtils.test.ts Co-authored-by: Antoine du Hamel --- tests/httpUtils.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index 12b6dcd0b..28ff81f64 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -79,6 +79,8 @@ describe(`http utils fetchUrlStream`, () => { it(`redirection with bad response`, async () => { await expect(fetchUrlStream(getUrl(301, 300))).rejects.toThrowError(); await expect(fetchUrlStream(getUrl(308, 199))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(301, 302))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(307))).rejects.toThrowError(); }); it(`rejects with error`, async () => { From 6fb8bf05bdbad21e345c67ba688c09cb09fa0112 Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Thu, 28 Dec 2023 09:41:37 +0800 Subject: [PATCH 14/14] fix eslint(arca/curly) in httpUtils.test.ts --- tests/httpUtils.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts index 28ff81f64..4676be3cf 100644 --- a/tests/httpUtils.test.ts +++ b/tests/httpUtils.test.ts @@ -16,7 +16,9 @@ describe(`http utils fetchUrlStream`, () => { if ([301, 302, 307, 308].includes(+statusCode)) { const redirectCode = parsedURL.searchParams.get(`redirectCode`)!; // mock response.headers.location - if (redirectCode) Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); + if (redirectCode) { + Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); + } } // handle request.on('error', err => ...)