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

Robustness of the "Goto Type Dialog" #7899

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Conversation

jtulach
Copy link
Contributor

@jtulach jtulach commented Oct 23, 2024

I started to see following exception when using the "GoTo Type Dialog" and the dialog stop showing anything:

java.lang.UnsupportedOperationException
        // deep stack skipped
        at org.netbeans.modules.lsp.client.bindings.TypeProviderImpl.computeTypeNames(TypeProviderImpl.java:54)
        at org.netbeans.modules.jumpto.type.GoToTypeAction$Worker.getTypeNames(GoToTypeAction.java:614)
        at org.netbeans.modules.jumpto.type.GoToTypeAction$Worker.run(GoToTypeAction.java:522)
        at org.openide.util.RequestProcessor$Task.run(RequestProcessor.java:1403)
        at org.netbeans.modules.openide.util.GlobalLookup.execute(GlobalLookup.java:45)
        at org.openide.util.lookup.Lookups.executeWith(Lookups.java:287)
        at org.openide.util.RequestProcessor$Processor.run(RequestProcessor.java:2018)

It is not good when a single broken provider brakes everything. This PR catches exceptions from each provider, logs the exception coming from TypeProvider.computeTypeNames and goes on and querying the other providers. With this fix I see the exception in the log, but _"Goto Type Dialog" continues to work as expected.

@jtulach jtulach self-assigned this Oct 23, 2024
@jtulach jtulach added kind:bug Bug report or fix Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) UI User Interface caused-by-plugin issue caused by a third party plugin labels Oct 23, 2024
@apache apache locked and limited conversation to collaborators Oct 23, 2024
@apache apache unlocked this conversation Oct 23, 2024
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@lahodaj lahodaj changed the base branch from master to delivery October 23, 2024 14:06
@lahodaj lahodaj added this to the NB24 milestone Oct 23, 2024
@jtulach jtulach merged commit 2237b71 into apache:delivery Oct 23, 2024
66 of 67 checks passed
@neilcsmith-net
Copy link
Member

neilcsmith-net commented Oct 23, 2024

@jtulach please read NOTICE emails. Only @ebarboni or @MartinBalin should be merging to delivery.

https://cwiki.apache.org/confluence/display/NETBEANS/Pull+requests+for+delivery

@mbien
Copy link
Member

mbien commented Oct 23, 2024

tbh it is easy to overlook when the branch target is changed by a reviewer. My inspector gadget senses tell me that @jtulach probably saw the approval and simply merged, thinking that the PR will land in master.

but there was likely no harm done here, since the branch was pre-version bump, so the merge commit didn't blow up.

@JaroslavTulach
Copy link

@jtulach please read NOTICE emails. Only @ebarboni or @MartinBalin should be merging to delivery.

https://cwiki.apache.org/confluence/display/NETBEANS/Pull+requests+for+delivery

Opps. I wanted to merge to master. Certainly I haven't changed the target branch!

lahodaj changed the base branch from master to delivery
easy to overlook when the branch target is changed by a reviewer.

I haven't noticed the change. I only checked Jan's message - I wasn't expecting target branch change.

likely no harm done here, since the branch was pre-version bump, so the merge commit didn't blow up.

Benefit of being behind the HEAD. Sorry for the ripples. @ebarboni or @MartinBalin feel free to keep or remove the change from delivery.

@ebarboni
Copy link
Contributor

it's fine, sync PR created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caused-by-plugin issue caused by a third party plugin Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:bug Bug report or fix UI User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants