fix: prevent repeated BadLocationException logging#1443
fix: prevent repeated BadLocationException logging#1443sebthom wants to merge 1 commit intoeclipse-lsp4e:mainfrom
Conversation
FlorianKroiss
left a comment
FlorianKroiss
left a comment
There was a problem hiding this comment.
Thanks for the quick PR @sebthom :)
| final int docLength = document.getLength(); | ||
| if (caretOffset < 0 || caretOffset > docLength) { | ||
| // Stale caret offset after document changes, skip this highlight run. | ||
| // Logging is limited to debug/trace to avoid noisy error log entries. | ||
| if (LanguageServerPlugin.DEBUG || LanguageServerPlugin.isLogTraceEnabled()) { | ||
| LanguageServerPlugin.logWarning( | ||
| "Ignoring documentHighlight for stale caret offset " + caretOffset + " (document length " //$NON-NLS-1$ //$NON-NLS-2$ | ||
| + docLength + ')'); | ||
| } | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
This is a lightweight version of what @rubenporras probably had in mind with checking LanguageServerWrapper.getTextDocumentVersion.
But do we actually gain something from such a check, besides a different error message being logged while debugging?
With or without this check, we are still in a race condition. So I would lean towards not performing this check at all.
There was a problem hiding this comment.
I thought my proposal was very simple, and will avoid the exceptions, @sebthom: do you think my proposal would not work? If it works, I would prefer that to this
There was a problem hiding this comment.
My proposal is https://github.com/eclipse-lsp4e/lsp4e/pull/1444/files
3 participants
Fixes #1439