feat: add remote-entry script resource retry for retry-plugin #3321
Conversation
🦋 Changeset detectedLatest commit: a2f18c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![squadronai[bot]](https://avatars.githubusercontent.com/in/901096?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
The key changes in this pull request are:
- Introduce a new
loadEntryErrorhook in theFederationHostclass to handle errors that may occur during the loading of remote entry scripts. This hook provides access to relevant information such as thegetRemoteEntryfunction, theFederationHostinstance, theRemoteInfoobject, theRemoteEntryExports, and aglobalLoadingobject. This allows for more robust error handling and potentially retrying the remote entry script loading process. - Add a try-catch block to the
getRemoteEntryfunction, which triggers a customloadEntryErrorevent in case of a failure. This event can be used to implement a retry mechanism or other error handling logic for remote-entry script resource retries.
These changes aim to improve the reliability and resilience of the remote entry script loading process, which is an important aspect of the federation-based architecture used in this project.
File Summaries
| File | Summary |
|---|---|
| packages/runtime/src/core.ts | The code changes introduce a new loadEntryError hook in the FederationHost class. This hook is designed to handle errors that may occur during the loading of remote entry scripts. The hook provides access to various relevant information, such as the getRemoteEntry function, the FederationHost instance, the RemoteInfo object, the RemoteEntryExports, and a globalLoading object. This addition allows for more robust error handling and potentially retrying the remote entry script loading process. |
| packages/runtime/src/module/index.ts | The code changes introduce a new feature to handle remote-entry script resource retries for the retry-plugin. The key modifications include adding a try-catch block to the getRemoteEntry function, which allows for error handling and triggering a custom loadEntryError event in case of a failure. This event can be used to implement a retry mechanism or other error handling logic. |
There was a problem hiding this comment.
Incremental Review
Comments posted: 3
Configuration
Squadron Mode: essential
Commits Reviewed
cafc5f040274ccb274f13d16910146415e8bd973...18b8abb96e41b8e94e32b7296edc1aa34af8f868
Files Reviewed
- packages/runtime/src/core.ts
- packages/runtime/src/module/index.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/small-eyes-change.md
- apps/router-demo/router-host-2000/rsbuild.config.ts
- apps/router-demo/router-host-2000/src/App.tsx
- packages/retry-plugin/src/index.ts
- packages/retry-plugin/src/util.ts
There was a problem hiding this comment.
Incremental Review
Comments posted: 5
Configuration
Squadron Mode: essential
Commits Reviewed
cafc5f040274ccb274f13d16910146415e8bd973...e23607305c2fe98c12cad9827cd295cca1e95b25
Files Reviewed
- packages/runtime/src/core.ts
- packages/runtime/src/module/index.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/small-eyes-change.md
- apps/router-demo/router-host-2000/rsbuild.config.ts
- apps/router-demo/router-host-2000/src/App.tsx
- packages/retry-plugin/src/index.ts
- packages/retry-plugin/src/util.ts
|
can you add new test cases for the feature? |
packages/runtime/src/module/index.ts
Outdated
| let remoteEntryExports; | ||
| const uniqueKey = getRemoteEntryUniqueKey(this.remoteInfo); | ||
| remoteEntryExports = | ||
| await this.host.loaderHook.lifecycle.getRemoteEntryExports.emit({ |
There was a problem hiding this comment.
it looks like the same as loadEntry hook
There was a problem hiding this comment.
Incremental Review
Comments posted: 5
Configuration
Squadron Mode: essential
Commits Reviewed
b9187f5c558c19679dfef7ba5b577c075bc69334...a2f18c98d3a91e1cb319f51a11e3f16bb333e375
Files Reviewed
- packages/runtime/src/core.ts
- packages/runtime/src/module/index.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/small-eyes-change.md
- apps/router-demo/router-host-2000/rsbuild.config.ts
- apps/router-demo/router-host-2000/src/App.tsx
- packages/retry-plugin/src/fetch-retry.ts
- packages/retry-plugin/src/index.ts
- packages/retry-plugin/src/types.d.ts
- packages/retry-plugin/src/util.ts
| [string, RequestInit], | ||
| Promise<Response> | void | false | ||
| >(), |
There was a problem hiding this comment.
The AsyncHook type parameters could be more descriptive. Consider using a type alias to better document the purpose of this hook:
| [string, RequestInit], | |
| Promise<Response> | void | false | |
| >(), | |
| type RemoteEntryRequestHookParams = [string, RequestInit]; | |
| type RemoteEntryRequestResult = Promise<Response> | void | false; | |
| requestRemoteEntry: new AsyncHook< | |
| RemoteEntryRequestHookParams, | |
| RemoteEntryRequestResult | |
| >(), |
packages/runtime/src/core.ts
Outdated
| getRemoteEntryExports: new AsyncHook< | ||
| [ | ||
| { | ||
| getRemoteEntry: typeof getRemoteEntry; |
There was a problem hiding this comment.
The loadEntryError hook's parameter structure appears to be incomplete and lacks type information. Consider adding proper type definitions for error handling:
| getRemoteEntryExports: new AsyncHook< | |
| [ | |
| { | |
| getRemoteEntry: typeof getRemoteEntry; | |
| loadEntryError: new AsyncHook< | |
| [{ | |
| error: Error; | |
| origin: string; | |
| retryCount?: number; | |
| }], | |
| void | |
| >(), |
packages/runtime/src/module/index.ts
Outdated
| const uniqueKey = getRemoteEntryUniqueKey(this.remoteInfo); | ||
| remoteEntryExports = | ||
| await this.host.loaderHook.lifecycle.getRemoteEntryExports.emit({ | ||
| getRemoteEntry, | ||
| origin: this.host, | ||
| remoteInfo: this.remoteInfo, |
There was a problem hiding this comment.
The initial getRemoteEntry call could benefit from more specific error typing and context. Consider adding a custom error type:
| const uniqueKey = getRemoteEntryUniqueKey(this.remoteInfo); | |
| remoteEntryExports = | |
| await this.host.loaderHook.lifecycle.getRemoteEntryExports.emit({ | |
| getRemoteEntry, | |
| origin: this.host, | |
| remoteInfo: this.remoteInfo, | |
| try { | |
| remoteEntryExports = await getRemoteEntry({ | |
| origin: this.host, | |
| remoteInfo: this.remoteInfo, | |
| remoteEntryExports: this.remoteEntryExports, | |
| }) as RemoteEntryExports; | |
| } catch (err: unknown) { |
packages/runtime/src/module/index.ts
Outdated
| uniqueKey, | ||
| }); | ||
|
|
||
| // get exposeGetter | ||
| if (!remoteEntryExports) { | ||
| remoteEntryExports = await getRemoteEntry({ | ||
| origin: this.host, | ||
| remoteInfo: this.remoteInfo, | ||
| remoteEntryExports: this.remoteEntryExports, | ||
| }); |
There was a problem hiding this comment.
The error recovery flow could benefit from a timeout to prevent hanging in case the error handler itself fails. Add a timeout wrapper:
| uniqueKey, | |
| }); | |
| // get exposeGetter | |
| if (!remoteEntryExports) { | |
| remoteEntryExports = await getRemoteEntry({ | |
| origin: this.host, | |
| remoteInfo: this.remoteInfo, | |
| remoteEntryExports: this.remoteEntryExports, | |
| }); | |
| const RECOVERY_TIMEOUT = 15000; // 15 seconds timeout | |
| remoteEntryExports = await Promise.race([ | |
| this.host.loaderHook.lifecycle.loadEntryError.emit({ | |
| getRemoteEntry, | |
| origin: this.host, | |
| remoteInfo: this.remoteInfo, | |
| remoteEntryExports: this.remoteEntryExports, | |
| globalLoading, | |
| uniqueKey, | |
| }), | |
| new Promise((_, reject) => | |
| setTimeout(() => reject(new Error('Error recovery timeout')), RECOVERY_TIMEOUT) | |
| ) | |
| ]); |
packages/runtime/src/module/index.ts
Outdated
| getRemoteEntry, | ||
| origin: this.host, | ||
| remoteInfo: this.remoteInfo, | ||
| remoteEntryExports: this.remoteEntryExports, |
There was a problem hiding this comment.
Add logging for debugging and monitoring purposes before entering error recovery:
| remoteEntryExports: this.remoteEntryExports, | |
| } catch (err) { | |
| console.warn(`Remote entry load failed for ${this.remoteInfo.name}, attempting recovery:`, err); |
There was a problem hiding this comment.
Incremental Review
Comments posted: 5
Configuration
Squadron Mode: essential
Commits Reviewed
a2f18c98d3a91e1cb319f51a11e3f16bb333e375...5607734548eaa04990479d4390c163bf403c1685
Files Reviewed
- packages/runtime/src/core.ts
- packages/runtime/src/module/index.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/small-eyes-change.md
- apps/router-demo/router-host-2000/rsbuild.config.ts
- apps/router-demo/router-host-2000/src/App.tsx
- packages/retry-plugin/src/fetch-retry.ts
- packages/retry-plugin/src/index.ts
- packages/retry-plugin/src/types.d.ts
- packages/retry-plugin/src/util.ts
| loadEntryError: new AsyncHook< | ||
| [ | ||
| { | ||
| getRemoteEntry: typeof getRemoteEntry; |
There was a problem hiding this comment.
The loadEntryError hook appears to be incomplete and missing critical error handling properties. Enhance it with a more comprehensive error interface:
| 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.
| remoteInfo: this.remoteInfo, | ||
| remoteEntryExports: this.remoteEntryExports, | ||
| }); | ||
| let remoteEntryExports; |
There was a problem hiding this comment.
The remoteEntryExports variable declaration could be more strictly typed to match its expected type. Add explicit typing:
| let remoteEntryExports; | |
| let remoteEntryExports: RemoteEntryExports | undefined; |
Description
feat: add remote-entry script resource retry for retry-plugin
Related Issue
Types of changes
Checklist