Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add remote-entry script resource retry for retry-plugin #3321

Merged
merged 7 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/small-eyes-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@module-federation/retry-plugin': patch
'@module-federation/runtime': patch
---

feat: add remote-entry script resource retry for retry-plugin
2 changes: 1 addition & 1 deletion apps/router-demo/router-host-2000/rsbuild.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default defineConfig({
shared: ['react', 'react-dom', 'antd'],
runtimePlugins: [
path.join(__dirname, './src/runtime-plugin/shared-strategy.ts'),
// path.join(__dirname, './src/runtime-plugin/retry.ts'),
path.join(__dirname, './src/runtime-plugin/retry.ts'),
],
// bridge: {
// disableAlias: true,
Expand Down
32 changes: 16 additions & 16 deletions apps/router-demo/router-host-2000/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@ init({
remotes: [],
plugins: [
BridgeReactPlugin(),
RetryPlugin({
fetch: {
url: 'http://localhost:2008/not-exist-mf-manifest.json',
fallback: () => 'http://localhost:2001/mf-manifest.json',
},
script: {
retryTimes: 3,
retryDelay: 1000,
moduleName: ['remote1'],
cb: (resolve, error) => {
return setTimeout(() => {
resolve(error);
}, 1000);
},
},
}),
// RetryPlugin({
danpeen marked this conversation as resolved.
Show resolved Hide resolved
// fetch: {
// url: 'http://localhost:2008/not-exist-mf-manifest.json',
// fallback: () => 'http://localhost:2001/mf-manifest.json',
// },
// script: {
// retryTimes: 3,
// retryDelay: 1000,
// moduleName: ['remote1'],
// cb: (resolve, error) => {
// return setTimeout(() => {
// resolve(error);
// }, 1000);
// },
// },
// }),
],
});

Expand Down
3 changes: 2 additions & 1 deletion packages/retry-plugin/src/fetch-retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ async function fetchWithRetry({
} catch (error) {
if (retryTimes <= 0) {
logger.log(
`>>>>>>>>> retry failed after ${retryTimes} times for url: ${url}, now will try fallbackUrl url <<<<<<<<<`,
`[ Module Federation RetryPlugin ]: retry failed after ${retryTimes} times for url: ${url}, now will try fallbackUrl url`,
);

if (fallback && typeof fallback === 'function') {
return fetchWithRetry({
url: fallback(url),
Expand Down
66 changes: 33 additions & 33 deletions packages/retry-plugin/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FederationRuntimePlugin } from '@module-federation/runtime/types';
import { fetchWithRetry } from './fetch-retry';
import { defaultRetries, defaultRetryDelay } from './constant';
import type { RetryPluginParams } from './types';
import { scriptCommonRetry } from './util';

const RetryPlugin: (params: RetryPluginParams) => FederationRuntimePlugin = ({
fetch: fetchOption,
Expand Down Expand Up @@ -36,39 +36,39 @@ const RetryPlugin: (params: RetryPluginParams) => FederationRuntimePlugin = ({
}
return fetch(url, options);
},
async getModuleFactory({ remoteEntryExports, expose, moduleInfo }) {
let moduleFactory;
const { retryTimes = defaultRetries, retryDelay = defaultRetryDelay } =
scriptOption || {};

if (
(scriptOption?.moduleName &&
scriptOption?.moduleName.some(
(m) => moduleInfo.name === m || (moduleInfo as any)?.alias === m,
)) ||
scriptOption?.moduleName === undefined
) {
let attempts = 0;

while (attempts - 1 < retryTimes) {
try {
moduleFactory = await remoteEntryExports.get(expose);
break;
} catch (error) {
attempts++;
if (attempts - 1 >= retryTimes) {
scriptOption?.cb &&
(await new Promise(
(resolve) =>
scriptOption?.cb && scriptOption?.cb(resolve, error),
));
throw error;
}
await new Promise((resolve) => setTimeout(resolve, retryDelay));
}
}
}
return moduleFactory;
async loadEntryError({
getRemoteEntry,
origin,
remoteInfo,
remoteEntryExports,
globalLoading,
uniqueKey,
}) {
if (!scriptOption) return;
const retryFn = getRemoteEntry;
const beforeExecuteRetry = () => delete globalLoading[uniqueKey];
const getRemoteEntryRetry = scriptCommonRetry({
scriptOption,
moduleInfo: remoteInfo,
retryFn,
beforeExecuteRetry,
});
return getRemoteEntryRetry({
origin,
remoteInfo,
remoteEntryExports,
});
},
async getModuleFactory({ remoteEntryExports, expose, moduleInfo }) {
if (!scriptOption) return;
const retryFn = remoteEntryExports.get;
const getRemoteEntryRetry = scriptCommonRetry({
scriptOption,
moduleInfo,
retryFn,
});
return getRemoteEntryRetry(expose);
},
});

Expand Down
8 changes: 8 additions & 0 deletions packages/retry-plugin/src/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { RemoteInfo } from '@module-federation/runtime/types';
export interface FetchWithRetryOptions {
url?: string;
options?: RequestInit;
Expand All @@ -24,3 +25,10 @@ export type RequiredFetchWithRetryOptions = Required<
Pick<FetchWithRetryOptions, 'url'>
> &
Omit<FetchWithRetryOptions, 'url'>;

export type ScriptCommonRetryOption = {
scriptOption: ScriptWithRetryOptions;
moduleInfo: RemoteInfo & { alias?: string };
retryFn: (...args: any[]) => Promise<any> | (() => Promise<any>);
beforeExecuteRetry?: (...args: any[]) => void;
};
47 changes: 47 additions & 0 deletions packages/retry-plugin/src/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { defaultRetries, defaultRetryDelay } from './constant';
import type { ScriptCommonRetryOption } from './types';
import logger from './logger';

export function scriptCommonRetry<T extends (...args: any[]) => void>({
scriptOption,
moduleInfo,
retryFn,
beforeExecuteRetry = () => {},
}: ScriptCommonRetryOption) {
return async function (...args: Parameters<T>) {
let retryResponse;
const { retryTimes = defaultRetries, retryDelay = defaultRetryDelay } =
scriptOption || {};
if (
(scriptOption?.moduleName &&
scriptOption?.moduleName.some(
(m) => moduleInfo.name === m || moduleInfo?.alias === m,
)) ||
scriptOption?.moduleName === undefined
) {
let attempts = 0;
while (attempts - 1 < retryTimes) {
try {
beforeExecuteRetry && beforeExecuteRetry();
retryResponse = await retryFn(...args);
break;
} catch (error) {
attempts++;
if (attempts - 1 >= retryTimes) {
scriptOption?.cb &&
(await new Promise(
(resolve) =>
scriptOption?.cb && scriptOption?.cb(resolve, error),
));
throw error;
}
logger.log(
`[ Module Federation RetryPlugin ]: script resource retrying ${attempts} times`,
);
await new Promise((resolve) => setTimeout(resolve, retryDelay));
}
}
}
return retryResponse;
};
}
18 changes: 17 additions & 1 deletion packages/runtime/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
InitTokens,
CallFrom,
} from './type';
import { getBuilderId, registerPlugins } from './utils';
import { getBuilderId, registerPlugins, getRemoteEntry } from './utils';
import { Module } from './module';
import {
AsyncHook,
Expand Down Expand Up @@ -115,6 +115,22 @@ export class FederationHost {
[string, RequestInit],
Promise<Response> | void | false
>(),
Comment on lines 115 to 117
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AsyncHook type parameters could be more descriptive. Consider using a type alias to better document the purpose of this hook:

Suggested change
[string, RequestInit],
Promise<Response> | void | false
>(),
type RemoteEntryRequestHookParams = [string, RequestInit];
type RemoteEntryRequestResult = Promise<Response> | void | false;
requestRemoteEntry: new AsyncHook<
RemoteEntryRequestHookParams,
RemoteEntryRequestResult
>(),

loadEntryError: new AsyncHook<
[
{
getRemoteEntry: typeof getRemoteEntry;
Comment on lines +118 to +121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loadEntryError hook appears to be incomplete and missing critical error handling properties. Enhance it with a more comprehensive error interface:

Suggested change
loadEntryError: new AsyncHook<
[
{
getRemoteEntry: typeof getRemoteEntry;
loadEntryError: new AsyncHook<
[{
error: Error;
origin: string;
retryCount: number;
retryLimit?: number;
retryDelay?: number;
}],

This provides better type safety and documentation for retry-related error handling parameters.

origin: FederationHost;
remoteInfo: RemoteInfo;
remoteEntryExports?: RemoteEntryExports | undefined;
globalLoading: Record<
string,
Promise<void | RemoteEntryExports> | undefined
>;
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved
uniqueKey: string;
},
],
Promise<(() => Promise<RemoteEntryExports | undefined>) | undefined>
>(),
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved
getModuleFactory: new AsyncHook<
[
{
Expand Down
31 changes: 23 additions & 8 deletions packages/runtime/src/module/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import {
RUNTIME_002,
runtimeDescMap,
} from '@module-federation/error-codes';
import { getRemoteEntry } from '../utils/load';
import { getRemoteEntry, getRemoteEntryUniqueKey } from '../utils/load';
import { FederationHost } from '../core';
import { RemoteEntryExports, RemoteInfo, InitScope } from '../type';
import { globalLoading } from '../global';

export type ModuleOptions = ConstructorParameters<typeof Module>[0];
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -34,18 +35,32 @@ class Module {
return this.remoteEntryExports;
}

// Get remoteEntry.js
const remoteEntryExports = await getRemoteEntry({
origin: this.host,
remoteInfo: this.remoteInfo,
remoteEntryExports: this.remoteEntryExports,
});
let remoteEntryExports;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remoteEntryExports variable declaration could be more strictly typed to match its expected type. Add explicit typing:

Suggested change
let remoteEntryExports;
let remoteEntryExports: RemoteEntryExports | undefined;

try {
remoteEntryExports = await getRemoteEntry({
origin: this.host,
remoteInfo: this.remoteInfo,
remoteEntryExports: this.remoteEntryExports,
});
} catch (err) {
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved
const uniqueKey = getRemoteEntryUniqueKey(this.remoteInfo);
remoteEntryExports =
await this.host.loaderHook.lifecycle.loadEntryError.emit({
getRemoteEntry,
origin: this.host,
remoteInfo: this.remoteInfo,
remoteEntryExports: this.remoteEntryExports,
globalLoading,
uniqueKey,
});
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved
}

assert(
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved
remoteEntryExports,
`remoteEntryExports is undefined \n ${safeToString(this.remoteInfo)}`,
);
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved

this.remoteEntryExports = remoteEntryExports;
this.remoteEntryExports = remoteEntryExports as RemoteEntryExports;
zhoushaw marked this conversation as resolved.
Show resolved Hide resolved
return this.remoteEntryExports;
}

Expand Down
Loading