Repository navigation
fix: guard against null ITextEditor document provider - #585
Draft
dungdong-aws wants to merge 1 commit into
Draft
dungdong-aws wants to merge 1 commit into
dungdong-aws wants to merge 1 commit into
Conversation
ITextEditor.getDocumentProvider() is optional in the Eclipse API and may return null for third party editors. AutoTriggerPartListener dereferenced it unconditionally on every editor activation, so the NPE escaped the IPartListener2 callback and the workbench showed a generic error dialog. Resolve the provider and document defensively in AutoTriggerPartListener (partActivated and onStart) and in QEclipseEditorUtils.getSelectionRange and insertAtCursor, degrading to a no-op instead of throwing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #510. An editor that implements
ITextEditorbut returnsnullfromgetDocumentProvider()makes Amazon Q raiseThe reporter sees the workbench's generic "An error has occurred. See error log for
more details." dialog simply by activating such an editor — no Amazon Q action is
required.
Root cause
IDocumentProvideris an optional part of the Eclipse text-editor API;ITextEditor.getDocumentProvider()is allowed to returnnull, and third partyeditors do.
AutoTriggerPartListener.partActivateddereferences it unconditionallyfor every activated
ITextEditor:Because
partActivatedis anIPartListener2callback, the NPE escapes into theworkbench, which reports it through its status handler — hence the dialog on every
activation.
onStart()in the same class already null-checked the document butnot the provider.
The repository already treats this API as nullable elsewhere —
AbapUtil(~221),AmazonQLspClientImpl(~505) andInlineChatSession(~109) all guard it — so thischange applies the existing in-repo convention to the paths that still miss it.
Changes
Production (~50 lines):
AutoTriggerPartListener: added a privateresolveDocument(ITextEditor)helperthat returns
nullwhen the editor, its provider, or its document isnull.partActivatednow returns early instead of attaching a listener, andonStartreuses the helper so a null provider re-arms the existing 1-second poll exactly
as a null document already did.
QEclipseEditorUtils: added an equivalent privategetDocument(ITextEditor)helper and routed
getSelectionRangeandinsertAtCursorthrough it; both nowreturn
Optional.empty()rather than throwing. These are the send-to-chat andinsert-at-cursor paths, reachable with the same editor.
Behaviour for editors that do have a document provider is unchanged.
The guards are deliberately local to each class rather than shared through a public
QEclipseEditorUtilshelper:QEclipseEditorUtilsis replaced bymockStaticinfour existing test classes, so routing
AutoTriggerPartListenerthrough it wouldmake the guard unobservable in that class's own unit tests.
Remaining call sites (intentionally not in this PR)
The same unchecked dereference exists in the inline-chat and agentic-edit paths —
InlineChatDiffManager(99, 100, 198, 199, 244),InlineChatUIManager(425) andAmazonQLspClientImpl(398, 400). Those are not part of this fix because they sitbehind
InlineChatSession.startSession, which already wraps the call intry/catchand ends the session, so they produce a handled failure rather than thereported workbench dialog. Keeping this PR to the unhandled path keeps it atomic
and reviewable; the inline-chat sites are worth a separate follow-up.
Edge cases covered
getDocumentProvider()returnsnull→ activation ignored, no listener attached,no active document recorded, and the subsequent
partDeactivatedis also a no-op.getDocument(input)returnsnull→ same no-op.onStartwith a null provider → retries on the existing timer instead of throwingon the UI thread.
null(already possible viagetActiveTextEditor()) → handled bythe same helper.
Tests / validation
AutoTriggerPartListenerTest#testPartActivateWithNullDocumentProviderand#testPartActivateWithNullDocument, both asserting no throw and that no documentlistener is attached or detached.
enforce a memory cgroup cap, so no build or test execution was performed. CI on
this PR (
mvn -B package, which also runs checkstyle withincludeTestSourceDirectory) is the first full validation.provider-less
ITextEditorwas built. The mechanism is established from sourceand from the exact exception message on the issue; the throw site is inferred from
the activation-only reproduction, since the issue quotes the message without a
stack trace.
Release target
Base
main(repository default; the Maven workflow runs for PRs targetingmain).Ticket