Skip to content

Commit 2f1ed77

Browse files
authored
Wait for Html buffer updates before making requests (#8748)
2 parents 5267161 + 604c555 commit 2f1ed77

File tree

2 files changed

+68
-3
lines changed

2 files changed

+68
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* File-based programs live directive diagnostics (PR: [#80575](https://github.com/dotnet/roslyn/pull/80575))
99
* Implement canonical miscellaneous files project loader for non-file-based programs (PR: [#80748](https://github.com/dotnet/roslyn/pull/80748))
1010
* Better handle if a BuildHost process crashes that prevents connection (PR: [#81041](https://github.com/dotnet/roslyn/pull/81041))
11+
* Wait for Html buffer updates before making requests (PR: [#8748](https://github.com/dotnet/vscode-csharp/pull/8748))
1112

1213
# 2.97.x
1314
* Add integration test for restore of file-based programs (PR: [#8470](https://github.com/dotnet/vscode-csharp/pull/8470))

src/lsptoolshost/razor/htmlDocumentManager.ts

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ import { UriConverter } from '../utils/uriConverter';
1717
export class HtmlDocumentManager {
1818
private readonly htmlDocuments: { [hostDocumentPath: string]: HtmlDocument } = {};
1919
private readonly contentProvider: HtmlDocumentContentProvider;
20+
private readonly pendingUpdates: {
21+
[documentPath: string]: {
22+
promise: Promise<void>;
23+
resolve: () => void;
24+
reject: (error: any) => void;
25+
};
26+
} = {};
2027

2128
private readonly razorDocumentClosedRequest: RequestType<TextDocumentIdentifier, void, Error> = new RequestType(
2229
'razor/documentClosed'
@@ -35,6 +42,20 @@ export class HtmlDocumentManager {
3542
}
3643

3744
public register() {
45+
const didChangeRegistration = vscode.workspace.onDidChangeTextDocument((e) => {
46+
// Check if this document is being monitored for updates
47+
if (e.document.uri.scheme === HtmlDocumentContentProvider.scheme) {
48+
const documentPath = getUriPath(e.document.uri);
49+
const pendingUpdate = this.pendingUpdates[documentPath];
50+
51+
if (pendingUpdate) {
52+
// Document has been updated, resolve the promise
53+
pendingUpdate.resolve();
54+
delete this.pendingUpdates[documentPath];
55+
}
56+
}
57+
});
58+
3859
const didCloseRegistration = vscode.workspace.onDidCloseTextDocument(async (document) => {
3960
// We log when a virtual document is closed just in case it helps track down future bugs
4061
if (document.uri.scheme === HtmlDocumentContentProvider.scheme) {
@@ -63,7 +84,7 @@ export class HtmlDocumentManager {
6384
this.contentProvider
6485
);
6586

66-
return vscode.Disposable.from(didCloseRegistration, providerRegistration);
87+
return vscode.Disposable.from(didChangeRegistration, didCloseRegistration, providerRegistration);
6788
}
6889

6990
public async updateDocumentText(uri: vscode.Uri, checksum: string, text: string) {
@@ -79,6 +100,17 @@ export class HtmlDocumentManager {
79100

80101
this.logger.logTrace(`New content for '${uri}', updating '${document.path}', checksum '${checksum}'.`);
81102

103+
// Create a promise for this document update
104+
let resolve: () => void;
105+
let reject: (error: any) => void;
106+
const updatePromise = new Promise<void>((res, rej) => {
107+
resolve = res;
108+
reject = rej;
109+
});
110+
111+
this.pendingUpdates[document.path] = { promise: updatePromise, resolve: resolve!, reject: reject! };
112+
113+
// Now update the document and fire the change event so VS Code will inform the Html language client.
82114
await vscode.workspace.openTextDocument(document.uri);
83115

84116
document.setContent(checksum, text);
@@ -92,6 +124,13 @@ export class HtmlDocumentManager {
92124
if (document) {
93125
this.logger.logTrace(`Removing '${document.uri}' from the document manager.`);
94126

127+
// Clean up any pending update promises for this document
128+
const pendingUpdate = this.pendingUpdates[document.path];
129+
if (pendingUpdate) {
130+
pendingUpdate.reject(new Error('Document was closed before update completed'));
131+
delete this.pendingUpdates[document.path];
132+
}
133+
95134
delete this.htmlDocuments[document.path];
96135
}
97136
}
@@ -111,10 +150,35 @@ export class HtmlDocumentManager {
111150
return undefined;
112151
}
113152

114-
// No checksum, just give them the latest document and hope they know what to do with it.
115-
116153
await vscode.workspace.openTextDocument(document.uri);
117154

155+
if (checksum) {
156+
// If checksum is supplied, that means we're getting this document because we're about to call an LSP method
157+
// on it. We know that we've got the right document, and we've been told about the right content by the server,
158+
// but all we can be sure of at this point is that we've fired the change event for it. The event firing
159+
// is async, and the didChange notification that it would generate is a notification, so doesn't necessarily
160+
// block. Before we actually make a call to the Html server, we should at least make sure that the document
161+
// update has been seen by VS Code. We can't get access to the Html language client specifically to check if it
162+
// has seen it, but we can trust that ordering will be preserved at least.
163+
const pendingUpdate = this.pendingUpdates[document.path];
164+
if (pendingUpdate) {
165+
try {
166+
// Wait for the update promise with a 5 second timeout
167+
await Promise.race([
168+
pendingUpdate.promise,
169+
new Promise((_, reject) =>
170+
setTimeout(() => reject(new Error('Document update timeout')), 5000)
171+
),
172+
]);
173+
} catch (error) {
174+
this.logger.logWarning(`Failed to wait for document update: ${error}`);
175+
} finally {
176+
// Clean up the promise reference
177+
delete this.pendingUpdates[document.path];
178+
}
179+
}
180+
}
181+
118182
return document;
119183
}
120184

0 commit comments

Comments
 (0)