From 8a3ff0acea0e654a8ba532016b23f8ca6c199dc2 Mon Sep 17 00:00:00 2001 From: Matthew Mallimo Date: Wed, 8 Mar 2023 19:32:54 -0500 Subject: [PATCH] fix(devCdn): requests were not using node-fetch API (#949) --- __mocks__/node-fetch.js | 15 +++++-- __tests__/server/utils/devCdnFactory.spec.js | 43 ++++++++++---------- src/server/utils/devCdnFactory.js | 32 ++++++--------- 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/__mocks__/node-fetch.js b/__mocks__/node-fetch.js index 7e53f3a7..416b303a 100644 --- a/__mocks__/node-fetch.js +++ b/__mocks__/node-fetch.js @@ -18,14 +18,23 @@ fetch.mockReturnJsonOnce = (obj) => { return fetch.mockImplementationOnce(() => Promise.reject(obj)); } - return fetch.mockImplementationOnce(() => Promise.resolve({ body: JSON.stringify(obj) })); + return fetch.mockImplementationOnce(() => Promise.resolve({ + json: () => Promise.resolve(obj), + text: () => Promise.resolve(JSON.stringify(obj)), + status: 200, + })); }; -fetch.mockReturnFileOnce = (body) => { +fetch.mockReturnFileOnce = (body, status = 200) => { if (body instanceof Error) { return fetch.mockImplementationOnce(() => Promise.reject(body)); } - return fetch.mockImplementationOnce(() => Promise.resolve({ body, statusCode: 200 })); + return fetch.mockImplementationOnce( + () => Promise.resolve({ + text: () => Promise.resolve(body), + status, + }) + ); }; module.exports = fetch; diff --git a/__tests__/server/utils/devCdnFactory.spec.js b/__tests__/server/utils/devCdnFactory.spec.js index 58b9e88e..7094b7f9 100644 --- a/__tests__/server/utils/devCdnFactory.spec.js +++ b/__tests__/server/utils/devCdnFactory.spec.js @@ -52,25 +52,7 @@ describe('one-app-dev-cdn', () => { }, }, }; - const defaultRemoteMap = { - key: '234234', - modules: { - 'module-b': { - node: { - url: 'https://example.com/cdn/module-b/1.0.0/module-b.node.js', - integrity: '123', - }, - browser: { - url: 'https://example.com/cdn/module-b/1.0.0/module-b.browser.js', - integrity: '234', - }, - legacyBrowser: { - url: 'https://example.com/cdn/module-b/1.0.0/module-b.legacy.browser.js', - integrity: '345', - }, - }, - }, - }; + let defaultRemoteMap; const defaultPublicDirContentsSetting = { moduleMapContent: JSON.stringify(defaultLocalMap), @@ -92,6 +74,7 @@ describe('one-app-dev-cdn', () => { if (!allowCacheWrite) { mkdirp(pathToCache, { mode: 444 }); } + const modulesDir = path.join(mockLocalDevPublicPath, 'modules'); mkdirp.sync(modulesDir); @@ -138,7 +121,25 @@ describe('one-app-dev-cdn', () => { jest .resetAllMocks() .resetModules(); - + defaultRemoteMap = { + key: '234234', + modules: { + 'module-b': { + node: { + url: 'https://example.com/cdn/module-b/1.0.0/module-b.node.js', + integrity: '123', + }, + browser: { + url: 'https://example.com/cdn/module-b/1.0.0/module-b.browser.js', + integrity: '234', + }, + legacyBrowser: { + url: 'https://example.com/cdn/module-b/1.0.0/module-b.legacy.browser.js', + integrity: '345', + }, + }, + }, + }; fetch.mockImplementation((url) => Promise.reject(new Error(`no mock for ${url} set up`))); }); @@ -629,7 +630,7 @@ describe('one-app-dev-cdn', () => { await fcdn.inject() .get('/module-map.json'); - fetch.mockReturnFileOnce(gotError); + fetch.mockReturnFileOnce('body', 501); const moduleResponse = await fcdn.inject() .get('/cdn/module-b/1.0.0/some-langpack.json'); expect(moduleResponse.statusCode).toBe(501); diff --git a/src/server/utils/devCdnFactory.js b/src/server/utils/devCdnFactory.js index fc6a7562..012efd4e 100644 --- a/src/server/utils/devCdnFactory.js +++ b/src/server/utils/devCdnFactory.js @@ -51,7 +51,7 @@ const consumeRemoteRequest = async (remoteModuleMapUrl, hostAddress, remoteModul // clear out remoteModuleBaseUrls as the new module map now has different urls in it // not clearing would result in an ever growing array remoteModuleBaseUrls.splice(0, remoteModuleBaseUrls.length); - const remoteModuleMap = JSON.parse(response.body); + const remoteModuleMap = await response.json(); const { modules } = remoteModuleMap; const oneAppDevStaticsAddress = `${hostAddress}/static`; Object.keys(modules).map((moduleName) => { @@ -135,13 +135,14 @@ export const oneAppDevCdnFactory = ({ // merge local with remote, with local taking preference oneAppDevCdn.get(`${routePrefix}/module-map.json`, async (req, reply) => { const hostAddress = useHost ? `http://${req.headers.host}` : `http://localhost:${process.env.HTTP_ONE_APP_DEV_CDN_PORT}`; - const localMap = useLocalModules ? JSON.parse(getLocalModuleMap({ pathToModuleMap: path.join(localDevPublicPath, 'module-map.json'), oneAppDevCdnAddress: hostAddress, })) : {}; - const remoteMap = remoteModuleMapUrl != null ? await consumeRemoteRequest(remoteModuleMapUrl, hostAddress, remoteModuleBaseUrls) : {}; // eslint-disable-line max-len + const remoteMap = remoteModuleMapUrl != null + ? await consumeRemoteRequest(remoteModuleMapUrl, hostAddress, remoteModuleBaseUrls) + : {}; // remoteMap always fulfilled const map = { ...remoteMap, @@ -166,22 +167,15 @@ export const oneAppDevCdnFactory = ({ remoteModuleBaseUrls ); const remoteModuleBaseUrlOrigin = new URL(knownRemoteModuleBaseUrl).origin; - try { - const remoteModuleResponse = await fetch(`${remoteModuleBaseUrlOrigin}/${incomingRequestPath}`, { - - headers: { connection: 'keep-alive' }, - agent: new ProxyAgent(), - }); - reply - .code(remoteModuleResponse.statusCode) - .type(path.extname(req.url)) - .send(remoteModuleResponse.body); - } catch (err) { - const status = err.statusCode === 'ERR_NON_2XX_3XX_RESPONSE' ? err.response.statusCode : 500; - return reply - .code(status) - .send(err); - } + const remoteModuleResponse = await fetch(`${remoteModuleBaseUrlOrigin}/${incomingRequestPath}`, { + headers: { connection: 'keep-alive' }, + agent: new ProxyAgent(), + }); + const { status, type } = remoteModuleResponse; + reply + .code(status) + .type(type) + .send(await remoteModuleResponse.text()); } else { reply .code(404)