From cd3a71a1e57ffdb1de4493843c7e9dd8843418b6 Mon Sep 17 00:00:00 2001 From: nameofname Date: Fri, 25 Apr 2025 15:27:28 -0400 Subject: [PATCH 1/2] fix(node): enhance error messaging for fetchAndRun with url --- .../node/src/__tests__/runtimePlugin.test.ts | 58 +++++++++++++++++-- packages/node/src/runtimePlugin.ts | 17 ++++-- packages/node/src/utils/flush-chunks.ts | 6 +- packages/node/src/utils/hot-reload.ts | 1 + packages/node/tsconfig.json | 1 + 5 files changed, 71 insertions(+), 12 deletions(-) diff --git a/packages/node/src/__tests__/runtimePlugin.test.ts b/packages/node/src/__tests__/runtimePlugin.test.ts index c0418b0151c..9c915783bec 100644 --- a/packages/node/src/__tests__/runtimePlugin.test.ts +++ b/packages/node/src/__tests__/runtimePlugin.test.ts @@ -41,9 +41,10 @@ jest.mock('vm', () => ({ }, })); -global.fetch = jest.fn().mockResolvedValue({ +(global as unknown as any).fetch = jest.fn().mockResolvedValue({ text: jest.fn().mockResolvedValue('// mock chunk content'), }); +const globalFetch = (global as unknown as any).fetch as jest.Mock; const mockWebpackRequire = { u: jest.fn((chunkId: string) => `/chunks/${chunkId}.js`), @@ -77,7 +78,7 @@ const mockNonWebpackRequire = jest.fn().mockImplementation((id: string) => { if (id === 'path') return require('path'); if (id === 'fs') return require('fs'); if (id === 'vm') return require('vm'); - if (id === 'node-fetch') return { default: global.fetch }; + if (id === 'node-fetch') return { default: globalFetch }; return {}; }); @@ -343,11 +344,11 @@ describe('runtimePlugin', () => { describe('fetchAndRun', () => { beforeEach(() => { - (global.fetch as jest.Mock).mockReset(); + (globalFetch as jest.Mock).mockReset(); }); it('should fetch and execute remote content', async () => { - (global.fetch as jest.Mock).mockResolvedValue({ + (globalFetch as jest.Mock).mockResolvedValue({ text: jest.fn().mockResolvedValue('// mock script content'), }); @@ -381,7 +382,7 @@ describe('runtimePlugin', () => { it('should handle fetch errors', async () => { const fetchError = new Error('Fetch failed'); - (global.fetch as jest.Mock).mockRejectedValue(fetchError); + (globalFetch as jest.Mock).mockRejectedValue(fetchError); const url = new URL('http://example.com/chunk.js'); const callback = jest.fn(); @@ -403,6 +404,9 @@ describe('runtimePlugin', () => { await new Promise((resolve) => setTimeout(resolve, 10)); expect(callback).toHaveBeenCalledWith(expect.any(Error), null); + expect(callback.mock.calls[0][0].message.includes(url.href)).toEqual( + true, + ); }); }); @@ -746,6 +750,50 @@ describe('runtimePlugin', () => { // The original promise should be reused expect(promises[0]).toBe(originalPromise); }); + + it('should delete chunks from the installedChunks when loadChunk fails', async () => { + // mock loadChunk to fail + jest + .spyOn(runtimePluginModule, 'loadChunk') + .mockImplementationOnce((strategy, chunkId, root, callback, args) => { + Promise.resolve().then(() => { + callback(new Error('failed to load'), undefined); + }); + }); + + jest + .spyOn(runtimePluginModule, 'installChunk') + .mockImplementationOnce((chunk, installedChunks) => { + // Mock implementation that doesn't rely on iterating chunk.ids + installedChunks['test-chunk'] = 0; + }); + + // Mock installedChunks + const installedChunks: Record = {}; + + // Call the function under test - returns the handler function, doesn't set webpack_require.f.require + const handler = setupChunkHandler(installedChunks, {}); + + const promises: Promise[] = []; + let res, err; + + try { + // Call the handler with mock chunk ID and promises array + handler('test-chunk', promises); + // Verify that installedChunks has test-chunk before the promise rejects + expect(installedChunks['test-chunk']).toBeDefined(); + res = await promises[0]; + } catch (e) { + err = e; + } + + // Verify that an error was thrown, and the response is undefined + expect(res).not.toBeDefined(); + expect(err instanceof Error).toEqual(true); + + // Verify the chunk data was properly removed + expect(installedChunks['test-chunk']).not.toBeDefined(); + }); }); describe('setupWebpackRequirePatching', () => { diff --git a/packages/node/src/runtimePlugin.ts b/packages/node/src/runtimePlugin.ts index 73ec86344ca..e7b19dd4388 100644 --- a/packages/node/src/runtimePlugin.ts +++ b/packages/node/src/runtimePlugin.ts @@ -2,6 +2,8 @@ import type { FederationRuntimePlugin, FederationHost, } from '@module-federation/runtime'; +import { Response } from 'node-fetch'; + type WebpackRequire = { (id: string): any; u: (chunkId: string) => string; @@ -125,7 +127,6 @@ export const loadFromFs = ( callback(new Error(`File ${filename} does not exist`), null); } }; - // Hoisted utility function to fetch and execute chunks from remote URLs export const fetchAndRun = ( url: URL, @@ -133,18 +134,22 @@ export const fetchAndRun = ( callback: (err: Error | null, chunk: any) => void, args: any, ): void => { - (typeof fetch === 'undefined' + const createFetchError = (e: Error) => + new Error(`Error while fetching from URL: ${url}`, { cause: e }); + (typeof (global as any).fetch === 'undefined' ? importNodeModule('node-fetch').then( (mod) => mod.default, ) - : Promise.resolve(fetch) + : Promise.resolve((global as any).fetch) ) .then((fetchFunction) => { return args.origin.loaderHook.lifecycle.fetch .emit(url.href, {}) .then((res: Response | null) => { if (!res || !(res instanceof Response)) { - return fetchFunction(url.href).then((response) => response.text()); + return fetchFunction(url.href).then((response: Response) => + response.text(), + ); } return res.text(); }); @@ -160,10 +165,10 @@ export const fetchAndRun = ( ); callback(null, chunk); } catch (e) { - callback(e as Error, null); + callback(createFetchError(e as Error), null); } }) - .catch((err: Error) => callback(err, null)); + .catch((err: Error) => callback(createFetchError(err as Error), null)); }; // Hoisted utility function to resolve URLs for chunks diff --git a/packages/node/src/utils/flush-chunks.ts b/packages/node/src/utils/flush-chunks.ts index c767a5a6d59..0017fe21fbf 100644 --- a/packages/node/src/utils/flush-chunks.ts +++ b/packages/node/src/utils/flush-chunks.ts @@ -1,5 +1,7 @@ /* eslint-disable no-undef */ +import { Response } from 'node-fetch'; + // @ts-ignore if (!globalThis.usedChunks) { // @ts-ignore @@ -121,7 +123,9 @@ const processChunk = async (chunk, shareMap, hostStats) => { let stats = {}; try { - stats = await fetch(statsFile).then((res) => res.json()); + stats = await (global as any) + .fetch(statsFile) + .then((res: Response) => res.json()); } catch (e) { console.error('flush error', e); } diff --git a/packages/node/src/utils/hot-reload.ts b/packages/node/src/utils/hot-reload.ts index 0297d069d5a..7971838ad27 100644 --- a/packages/node/src/utils/hot-reload.ts +++ b/packages/node/src/utils/hot-reload.ts @@ -2,6 +2,7 @@ import { getAllKnownRemotes } from './flush-chunks'; import crypto from 'crypto'; import helpers from '@module-federation/runtime/helpers'; import path from 'path'; +import { Response } from 'node-fetch'; declare global { var mfHashMap: Record | undefined; diff --git a/packages/node/tsconfig.json b/packages/node/tsconfig.json index c0371e6f17e..f0480c66410 100644 --- a/packages/node/tsconfig.json +++ b/packages/node/tsconfig.json @@ -2,6 +2,7 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { "target": "ES2021", + "lib": ["es2022.error"], "moduleResolution": "node", "resolveJsonModule": true, "skipLibCheck": true, From 1a881c089a226bde63e0deadb594c5f88ec98024 Mon Sep 17 00:00:00 2001 From: nameofname Date: Fri, 25 Apr 2025 16:46:23 -0400 Subject: [PATCH 2/2] chore(node): added minor version changeset --- .changeset/late-maps-exist.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/late-maps-exist.md diff --git a/.changeset/late-maps-exist.md b/.changeset/late-maps-exist.md new file mode 100644 index 00000000000..55bad8ef76e --- /dev/null +++ b/.changeset/late-maps-exist.md @@ -0,0 +1,5 @@ +--- +'@module-federation/node': minor +--- + +enhance error messaging for fetchAndRun with url