amondel2 commented on code in PR #441:
URL:
https://github.com/apache/grails-intellij-plugin/pull/441#discussion_r4290191707
##########
plugin/src/main/java/org/apache/grails/intellij/plugin/projectView/nodes/GrailsApplicationNode.java:
##########
@@ -50,7 +52,21 @@ public boolean shouldUpdateData() {
public @NotNull Collection<AbstractTreeNode<?>> getChildren() {
Collection<AbstractTreeNode<?>> result = new ArrayList<>();
for (GrailsViewNodeProvider provider :
GrailsViewNodeProvider.EP_NAME.getExtensionList()) {
- result.addAll(provider.createNodes(getValue(), getSettings()));
+ try {
+ result.addAll(provider.createNodes(getValue(), getSettings()));
+ }
+ catch (RuntimeException | LinkageError e) {
+ // The pane builds children on a background thread and a throw there
aborts the whole repaint,
+ // blanking the Grails view. One broken provider must not take the
others down with it.
+ // Control-flow exceptions are the exception to that: the platform
cancels a background read
+ // like this when a write action becomes pending and retries the build
afterwards, so
+ // swallowing one strands the pane on "Loading...". Rethrow those
untouched.
+ if (e instanceof ControlFlowException) {
+ throw e;
+ }
+ Logger.getInstance(GrailsApplicationNode.class)
+ .warn("Grails view provider " + provider.getClass().getName() + "
failed; skipping its nodes", e);
Review Comment:
`LOG.error` now, and `testFailingProviderIsLoggedAsError` expects it through
`LoggedErrorProcessor` — which does not record a `warn`, so the test fails
at the old level.
`testThrowingProviderDoesNotSuppressSiblingNodes` pins that the pane still
builds around the failure.
You were right that both changes alter behaviour for every provider, and the
PR description no longer
says "purely additive". It now has a *Behaviour changes beyond the custom
folders feature* section
calling out both:
- provider isolation in `GrailsApplicationNode.getChildren()` — one provider
that throws is caught
(rethrowing `ControlFlowException` untouched) and its nodes skipped,
instead of the throw escaping the
background children build and blanking the whole view;
- `getAppRoot()` null-safety in `GrailsAppNodeProvider` and
`GrailsViewItems` — `grails-app` is now
looked up under the application root and absent renders nothing, instead
of throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
