OCPBUGS-86280: Guard against null plugin route components#16587
Conversation
Validate that a plugin route component resolves to a function before passing it to React.lazy(). When a dynamic plugin fails to load its component (resolves to null/undefined), throw an error caught by the existing ErrorBoundaryPage instead of crashing with React error openshift#306. Also add missing pluginName to useMemo dependency array. Assisted-by: 🤖 Claude Opus/Sonnet 4.6
|
@cardil: This pull request references Jira Issue OCPBUGS-86280, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughLazyRoutePage's lazy component resolution now validates that the resolved ChangesPlugin route lazy component validation
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cardil The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/jira refresh |
|
@cardil: This pull request references Jira Issue OCPBUGS-86280, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@cardil: This pull request references Jira Issue OCPBUGS-86280, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@cardil: This pull request references Jira Issue OCPBUGS-86280, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@cardil: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Analysis / Root cause:
When a dynamic plugin's route component fails to load (resolves to null/undefined instead of a React component),
React.lazy()receives{ default: null }and crashes with React error #306: "Element type is invalid. Received a promise that resolves to: null. Lazy element type must resolve to a class or function."This can happen when a plugin pod is not running, not reachable, or when module federation fails silently after a cluster upgrade.
Solution description:
Add a type check in
LazyRoutePagebefore passing the resolved component toReact.lazy(). If the component is not a function, throw an error with the plugin name and extension UID. This error is caught by the existingErrorBoundaryPageinapp-contents.tsx, showing the standard "Something wrong happened" + "Reload page" UI instead of crashing.Also fixes a missing
pluginNamein theuseMemodependency array.Screenshots / screen recording:
N/A — defensive fix, no visual changes in the happy path.
Test setup:
Requires a dynamic plugin whose route component resolves to null. This can be simulated by registering a ConsolePlugin that points to a non-existent or broken plugin service.
Test cases:
Browser conformance:
Additional info:
console.warnincoderef-resolver.tsfor null codeRefs is intentional — 43 extension properties have optional codeRefs where null is legitimate. The guard belongs at the consumer level (usePluginRoutes.tsx) wherecomponentis known to be required.Assisted-by: 🤖 Claude Opus/Sonnet 4.6
Summary by CodeRabbit
Release Notes