Skip to content

fix: avoid BadLocationException by not working on out of date documents#1444

Merged
rubenporras merged 1 commit into
eclipse-lsp4e:mainfrom
rubenporras:hoveTimestamp
Dec 2, 2025
Merged

fix: avoid BadLocationException by not working on out of date documents#1444
rubenporras merged 1 commit into
eclipse-lsp4e:mainfrom
rubenporras:hoveTimestamp

Conversation

@rubenporras
Copy link
Copy Markdown
Contributor

@rubenporras rubenporras commented Dec 2, 2025

Fixes #1439

@rubenporras rubenporras requested a review from sebthom December 2, 2025 09:32
Comment on lines +211 to +215
long currentTimestamp = DocumentUtil.getDocumentModificationStamp(document);

if (currentTimestamp != timestamp) {
return;
}
Copy link
Copy Markdown
Contributor

@FlorianKroiss FlorianKroiss Dec 2, 2025

Choose a reason for hiding this comment

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

This check is probably cheaper to perform, but since this method runs in a job, it is still possible that the document is changed after this check and we get a BadLocationException afterwards.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@FlorianKroiss , you are right. What about now? I think we can check once before calculating the position to avoid creating potentially wrong highlights, and once more in the exception case to be really sure we do not have the exception because of a change in document.

We could also skip the first check, but I think it is very cheap, so better have it than sending outdated request to the LS.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes that should work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@sebthom , I am merging this one and making it also part of the release with fixes.

@rubenporras rubenporras merged commit 8747384 into eclipse-lsp4e:main Dec 2, 2025
11 checks passed
@rubenporras rubenporras deleted the hoveTimestamp branch December 2, 2025 10:35
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.

Typing at the end of the document occassionally logs BadLocationException

2 participants