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

Add error handling to extension load #13204

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

nwmac
Copy link
Member

@nwmac nwmac commented Jan 24, 2025

Summary

Fixes #13205

Technical notes summary

This PR improves the error handling when we load extensions.

I checked out the elemental-ui extension with the 2.9 branch, built it and served it up locally and developer loaded it into a 2.11 UI running locally - to reproduce trying to load a Vue 2 extension into Vue 3.

We don't have any exception handling in the load - the script for the extension loads, so our onloadhandler gets called. The script has an exception when it is evaluated at load time (error reading 'extend'), but this does not affect our code path - we check window[id]` is set, but because the injected script errored, it is an empty object and does not have the default function, so the real error for us is when we call that method here - https://github.com/rancher/dashboard/blob/master/shell/core/plugins.js#L116 - this throws an unhandled exception.

In the PR, I have added a check for the default function and added a try/catch around the call of it.

With this, the UI handles the load error. Note when running in dev, you will get the red overlay saying there is an unhandled exception (this is due to the error with the script load), but if you close it, our UI is working fine - before, you'd be stuck with the loading spinner. In production you wont' get the red overlay.

Areas or cases that should be tested

Extension loading

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@nwmac nwmac marked this pull request as ready for review January 24, 2025 13:44
@nwmac nwmac requested a review from rak-phillip January 24, 2025 13:44
@rancher-ui-project-bot rancher-ui-project-bot bot added this to the v2.11.0 milestone Jan 24, 2025
@nwmac nwmac requested review from richard-cox and aalves08 and removed request for aalves08 January 24, 2025 21:01
@aalves08 aalves08 requested a review from jordojordo January 27, 2025 10:17
jordojordo
jordojordo previously approved these changes Jan 30, 2025
Copy link
Member

@jordojordo jordojordo left a comment

Choose a reason for hiding this comment

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

lgtm - I was unable to trigger the "Could not initialize plugin" error but I did test with multiple incompatible extensions and noticed I could still navigate around without the window[id].default is not a function error.

@nwmac nwmac force-pushed the improve-extension-load-error-handling branch from 384da16 to e8d2c35 Compare January 31, 2025 10:53
@nwmac nwmac requested a review from jordojordo January 31, 2025 10:53
@nwmac
Copy link
Member Author

nwmac commented Jan 31, 2025

@jordojordo I had to rebase and other changes went in to remove extension unload. Can you eyeball again and approve if okay. Thanks

Copy link
Member

@jordojordo jordojordo left a comment

Choose a reason for hiding this comment

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

lgtm

@nwmac nwmac merged commit 56fbea3 into rancher:master Jan 31, 2025
32 checks passed
@nwmac nwmac deleted the improve-extension-load-error-handling branch January 31, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors are not handled on extension load
2 participants