delayOpenNotifications=true results in the wrong document version and content being sent in didOpen notification

I enabled delayOpenNotifications in the pre-release version of my extension but a bug came up that it was corrupting the servers overlays for open files in some cases.

I was able to reproduce this, and it seems to happen when code actions create new files with contents. The delayed didOpen notification gets sent with the "updated" content/version of the file instead of what it would've been had the notification not been delayed.

For example given a code action that has:

"edit": {
    "documentChanges": [
        {
            "kind": "create",
            "uri": "file:///D:/foo/foo.dart"
        },
        {
            "edits": [
                {
                    "newText": "part of 'sort_declarations.dart';\r\n\r\n",
                    "range": { "end": { "character": 0, "line": 0 }, "start": { "character": 0, "line": 0 } }
                }
            ],
            "textDocument": { "uri": "file:///D:/foo/foo.dart", "version": null }
        }
    ]
},

Without delayed notifications, this results in the following LSP traffic, which opens the file (empty) and then applies the edit:

==> {"method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///D%3A/foo/foo.dart","languageId":"dart","version":1,"text":""}}}
==> {"method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///D%3A/foo/foo.dart","version":2},"contentChanges":[{"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},"rangeLength":0,"text":"part of 'sort_declarations.dart';\r\n\r\n"}]}}

However, with delayed notifications enabled, we get a didOpen that has the modified content in it, and then the didChange notification re-inserts the same text, resulting in duplication of content in the servers version of the file:

==> {"method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///D%3A/foo/foo.dart","languageId":"dart","version":2,"text":"part of 'sort_declarations.dart';\r\n\r\n"}}}
==> {"method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///D%3A/foo/foo.dart","version":2},"contentChanges":[{"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},"rangeLength":0,"text":"part of 'sort_declarations.dart';\r\n\r\n"}]}}

Disable delayOpenNotifications due to bug

Add failing/skipped test for issue microsoft#1695

I've opened a PR with a failing test for this (and a request for some input) here:

#1696

Add failing/skipped test for issue #1695 (#1696)

Actually I tend to remove that feature again since it never shipped in a stable release. I looked through the code and it is almost not fixable since VSCode has no onWillChangeTextDocument notification. The problem is that when we sent an open notification there are events send out and they carry the TextDocument and we can't fake the content of a TextDocument to a previous content which we would need to send the right content to the server.

I will think about it more but delaying open notifications might not be a good idea.

The problem is that when we sent an open notification there are events send out and they carry the TextDocument and we can't fake the content of a TextDocument to a previous content which we would need to send the right content to the server.

I'm not sure I understand. Why can't you just capture the even that you would send during onDidOpenTextDocument and then send it later?

I will think about it more but delaying open notifications might not be a good idea.

Honestly I agree, however VS Code's behaviour of sending didOpen/didClose when you hover over things is absolutely bananas IMO. If VS Code won't fix it and it's not implemented here, then language servers will try to handle it (we want to skip doing any work we'd usually do when a document opens if it's not really been opened) and that's the worst place to put handling for VS Code-specific quirks IMO.

You can write code like this:

let feature: DidOpenTextDocumentFeature = client.getFeature(...);
feature.onNotificationSent(event => {
	event.textDocument;
});

And there the textDocument property of the event is of type TextDocument. TextDocuments in the VS Code extension host are live objects so you can't capture their state and hence I can't send this event when I sent the open to the language server with the right content. This API exists since a long time and sending something else will only be confusing if delay open notification is enabled.

Why are you already doing work on the server side on open? And what kind of work is this.

Why are you already doing work on the server side on open? And what kind of work is this.

I can answer part of this at least. This happens because we have part files in Dart that share private declarations. We have a quick fix in the main file for creating and moving to (opening) the (currently) non-existing part file. When it gets created, we must fill its content with at least part of 'main_file.dart';, so they get linked correctly (as you can see on the payload above).

Why are you already doing work on the server side on open? And what kind of work is this.

In some situations, we only analyze content when a file is open (this was done to support some cases where users opened huge folders and didn't want the entire workspace analyzed). This means when a file is opened, if that file is not already part of an analysis context, we will create one and start analyzing that project (to produce diagnostics).

I think it's also possible that if VS Code is normalizing newlines, then the open event could contain different newlines to what the server already has, and therefore the event will be a modification, also triggering analysis.

In your example above, couldn't you have feature.onNotificationSent just fire normally, and only apply the delay over-the-wire? Really all we're trying to avoid is a pair of open/close events being sent together. If anything happens immediately after the open that is not just a matching close event, it should just go as normal. Otherwise, both events should just be dropped.

Or, do you think VS Code could be convinced to stop leaking their internal model open/closes to something like LSP which is supposed to represent a change of ownership? It feels completely wrong for servers (or the LSP client IMO) to be handling this. 😔

Felipe Morschel (@FMorschel)

Felipe Morschel (@FMorschel) ~~that case actually doesn't matter here, because in that case the file will also be modified~~ (see edit). We're strictly trying to avoid the open+close pairs of events that VS Code triggers when you hold Ctrl and hover over any symbol (VS Code "reads" the file to show a preview in the tooltip, which triggers an open followed by an immediate close).

Edit: To be clear, the issue you describe is related to the bug with this feature (the notification being sent with the wrong content), but the question is about why we want this feature at all.

Fire feature.onNotificationSent right away is IMO not a good idea if we then drop the open event. In that case we would need to fake a onNotificationSent for the DidCloseTextDocumentFeature.

The only problems we have seen so far with the unnecessary open/close pair are around diagnostics. To overcome those the client supports pull diagnostics which takes care of not pulling for things that are not visible in the UI.

I am still not a fan of adding the delay open notification support since I am sure there are other cases we haven't thought of and then we might not be able to fix them.

In that case we would need to fake a onNotificationSent for the DidCloseTextDocumentFeature.

You could just send emit the "real" close event that caused the open event to be dropped? (eg. both get dropped over the wire, but both events are still emitted normally as events)?

The only problems we have seen so far with the unnecessary open/close pair are around diagnostics.

The thing I was trying to improve was performance - not starting unnecessary work because we believe the user has opened a file when they have not. There can be a lot of work done when analyzing a project that isn't just producing diagnostics, it might also include building indexes so that other requests (like Find References) are fast.

To overcome those the client supports pull diagnostics which takes care of not pulling for things that are not visible in the UI.

Honestly, I don't know how pull diagnostics are supposed to work. When a user opens a Dart project, they want to immediately see the diagnostics for the whole project. I'm not sure how this would work if diagnostics are pulled.

I am still not a fan of adding the delay open notification support since I am sure there are other cases we haven't thought of and then we might not be able to fix them.

Maybe - but maybe not :) It's an opt-in flag, and it could be marked as experimental. The only way to find issues is by using it (which is what I did - my pre-release extension users have had it enabled for weeks, and this is the only issue that's come up so far).

On the surface the problem seems fairly simple, but I think the current implementation is more complex than I imagined. All I really want is a small change to the over-the-wire data:

When there's an outgoing message:
- If it is a didOpen message, add it to a queue instead of sending it immediately
- If it is a didClose message, check if the last message on the queue is a corresponding didOpen. If so, drop the close and remove the open from the queue, otherwise flush the queue then send the close message immediately
- For any other message, flush the queue, and immediately send the message

The only thing missing from that is that a newly-opened document might not be sent if there isn't another request sent afterwards. That seems unlikely, but to handle that, we could:

When any new document is "really opened" (using the new tabs API), just flush the queue
and/or; 10ms after adding a didOpen to the queue, automatically send it (I think the open/close events are basically synchronous so they always arrive atomically)

I think this change could be small and fairly easy to validate.

I completely get that this is weird and it shouldn't be needed here, but it belongs in servers even less. We shouldn't have to change things in the server to try and guess when this VS Code-specific behaviour is happening and avoid doing work we would otherwise do. The LSP spec describes this event as the document being opened and now managed by the client. I don't think anyone reading the LSP spec would assume that "the editor reading a file off the disk to see its contents" would trigger a pair of open/close events and therefore they should avoid doing any work when they get an open event.

Revert to stable vscode-languageclient v9

Dirk Bäumer (@dbaeumer)

Dirk Bäumer (@dbaeumer) have you any further thoughts on this? This flag is still in the current codebase and broken.

I've been thinking about just using a fork of this client that has a basic implementation to drop consecutive open/close notifications if nothing else comes in-between (with a timer that flushes the open after a few ms) but it would be much nicer to be able to use official builds.

The current implementation basically does what you suggest (see

vscode-languageserver-node/client/src/common/textSynchronization.ts

Line 65 in 02766a9

this._pendingOpenNotifications = new Map<string, TextDocument>();

) minus the fact that it sends the wrong content on open. I again looked into sending a fake feature.onNotificationSent and to capture the state of the document for the real open, however it introduces another inconsistency. The middleware will not receive the open event although feature.onNotificationSent did fire.

Instead of tweaking the open / close events further I would really like to understand why the current pull diagnostic implementation doesn't help in your case. I supports both pulling for workspace diagnostics with a priorization of the visible files. Are there other requests where you would need the information about which files are visible? In general using pull fixed the problem we saw in VS Code and our extensions with the unnecessary open / close events.

If you have tweaked the current implementation to fix the wrong content on open I would be happy to look at the change to see if I make things to complicated.

All I am trying to say is that you can use pull diagnotics as an indication that work is requested for a document that is visible to the users. I know that this has a bad name so we could have another request initiateWork that fires on the same logic than pull diagnostic does.

But there is no opposite of this for when a document is not visible. For example, let's say that I only wanted to analyze projects that have an open file, and I want to cache the analysis results for those open files (to ensure all requests for things like code completion are fast, without using too much memory by caching ASTs for all files).

Today:

in didOpen, compute the surrounding project for the open file, and if it's not already in the set of analyzed projects, add it
if the project was not previously analyzed, analyzed it
in didClose, remove the project from the set of analyzed projects

If we stopped using didOpen and instead used diagnostics, what would we replace 3 with?

Though it's slightly missing the main point here. VS Code is doing something that IMO nobody would expect from reading the LSP spec. VS Code is leaking its internal data model into another process for no obvious reason. There is zero benefit to an LSP server getting these events. IMO the didOpen/didClose LSP implementation should just be rewritten against the tab events and not the document events at all. Generic LSP servers shouldn't have to change to work around this odd behaviour that is only from VS Code.

TS Go moves to LSP and they use pull diagnostics and it works for them as expected. The encountered some problems around missing pulls and missing visible documents which I all fixed for them.

I haven't used the Go version, but FWIW the third highest-voted issue in the VS Code repo is complaints about diagnostics not showing for the whole project and appearing/disappearing as you open/close files (microsoft/vscode#13953). I see this almost every day, and it is infuriating. I'm not sure TS is a good example of why pull-diagnostics is a good model.

As said before if you want to provide a PR that works around all the things mentioned before I am happy to look into it. However I would always offer it behind an experimental flag.

Sure, I will put something together (although rather than my original idea above, I might try directly using the tab APIs, because I think that would be simpler than adding a queue etc.). If you don't want to accept it, at this point I think I'd be happy to just use a fork (it might simplify some other things anyway). I'm just not convinced that making changes in the server makes sense here.

Thanks!

Dirk Bäumer (@dbaeumer)

Dirk Bäumer (@dbaeumer) I started looking at this again, and swapping to the tabs API entirely is awkward because of the change event. So I'm thinking about just making a smaller fix for the actual issue here.

How would you feel about changing the arguments for the didOpen middleware to not pass the TextDocument, but instead just a snapshot of the components (eg. the uri, language, string contents) or even the pre-computed DidOpenTextDocumentParams?

I feel like the issue here is the capturing of TextDocument which will always be risky if it does not get passed along synchronously (for example if you use async middleware for didOpen, you would introduce the same bug described here).

How would you feel about changing the arguments for the didOpen middleware to not pass the TextDocument, but instead just a snapshot of the components (eg. the uri, language, string contents) or even the pre-computed DidOpenTextDocumentParams?

I am not a fan if we do this for a single middleware call. It will make things inconsistent. We discussed that problem a while back and the solution we had in mind was to implement TextDocument as a readonly document canpturing the state when we receive the event. This introduces an identity problem but I think it is not as disruptive as changing all the middleware signatures.

We discussed that problem a while back and the solution we had in mind was to implement TextDocument as a readonly document canpturing the state when we receive the event

Presumably methods like save() could just throw - but what about things like positionAt()? Would they have to be reimplemented, or could they be reused from somewhere?

I think it is not as disruptive as changing all the middleware signatures

I agree it's less disruptive for users, I'm not sure it's better because:

Having a class that claims to be a TextDocument but throws if you try to use it as one moves errors from dev time to runtime
Probably almost nobody is implementing middleware over didOpen/didClose (if they did, and it's not synchronous, they'd hit this bug)
It could be a major semver version bump.. the few people in the world (if anyone) that has this middleware can choose when to upgrade, and it's a trivial fix

Another option could be to say that middleware always fires immediately (and therefore gets the real TextDocument), but then we capture the text into _pendingOpenNotifications. This avoids any signature changes or readonly implementations, it just means that the middleware might fire in the case where the notification gets dropped. As long as we make sure the close middleware still fires, I don't think this would be bad (it can be documented as behaviour of delayOpenNotifications)?

Dirk Bäumer (@dbaeumer)

Dirk Bäumer (@dbaeumer) any thoughts on the above?

Danny Tuppeny (@DanTup)

Danny Tuppeny (@DanTup) sorry for the delay but I am actually out on my well deserved sabbatical until beginning of Mai.

save() returns to my knowledge a boolean and could simply return false.
all the other methods are IMO easy to implement using the vscode-textdocument package. There might be some methods missing but I thing the can easily be filled in using AI.

I am not a big fan of always firing the middleware.

Dirk Bäumer (@dbaeumer)

Dirk Bäumer (@dbaeumer)

all the other methods are IMO easy to implement using the vscode-textdocument package

Oh, I didn't know about that package.
If you think that's the right direction, I might try and have a go at it in the coming weeks.

sorry for the delay but I am actually out on my well deserved sabbatical until beginning of Mai.

Ah, no problem at all - enjoy! 🙂

Snapshot TextDocuments when delayOpenNotifications=true to avoid serv…

Dirk Bäumer (@dbaeumer)

Dirk Bäumer (@dbaeumer) no rush, just for when you are back :-)

I opened a PR at #1755 that I think solves the issue in the way you described above. Unfortunately it involves more implementation of TextDocument methods than I expected and it feels bad to re-implement them here, but I've added a comment on the PR calling this out and will await your feedback.

Fix is here: #1773

Dirk Bäumer (@dbaeumer)

Closing this as #1773 was merged.

Dirk Bäumer (@dbaeumer) are you planning on doing another -next release before the stable release or is the stable release near?

I did a next release today:

vscode-languageserver@10.0.0-next.18
vscode-languageclient@10.0.0-next.22

Dirk Bäumer (@dbaeumer)

Dirk Bäumer (@dbaeumer) great, thank you!

Navigation Menu

Repository navigation

delayOpenNotifications=true results in the wrong document version and content being sent in didOpen notification #1695

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

delayOpenNotifications=true results in the wrong document version and content being sent in didOpen notification #1695

Description

Activity

DanTup commented on Dec 9, 2025

dbaeumer commented on Dec 16, 2025

DanTup commented on Dec 16, 2025

dbaeumer commented on Dec 17, 2025

FMorschel commented on Dec 17, 2025

DanTup commented on Dec 17, 2025

DanTup commented on Dec 17, 2025

dbaeumer commented on Dec 18, 2025

DanTup commented on Dec 18, 2025

DanTup commented on Feb 12, 2026

dbaeumer commented on Feb 17, 2026

4 remaining items

DanTup commented on Feb 25, 2026

DanTup commented on Feb 25, 2026

dbaeumer commented on Feb 26, 2026

DanTup commented on Feb 26, 2026

DanTup commented on Mar 16, 2026

dbaeumer commented on Apr 1, 2026

DanTup commented on Apr 1, 2026

DanTup commented on Apr 22, 2026

dbaeumer commented on May 22, 2026

DanTup commented on May 26, 2026

dbaeumer commented on May 27, 2026

DanTup commented on May 27, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions