Skip to content

Commit 88beb2e

Browse files
MaxKlessMaxKless
committed
test(nxls): fix hanging nxls in watcher
1 parent 09130a8 commit 88beb2e

1 file changed

Lines changed: 208 additions & 85 deletions

File tree

‎apps/nxls-e2e/src/nxls-wrapper.ts‎

Lines changed: 208 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export class NxlsWrapper {
2828
string,
2929
[(params: object | any[] | undefined) => void, NodeJS.Timeout]
3030
>();
31+
private communicationHealthy = true;
3132
private earlyExitListener = (code: number) => {
3233
console.log(`nxls exited with code ${code}`);
3334
console.log(`nxls stderr: ${this.process?.stderr?.read()}`);
@@ -98,62 +99,40 @@ export class NxlsWrapper {
9899
}
99100

100101
async stopNxls(version?: string) {
101-
await this.sendRequest({
102-
method: 'shutdown',
103-
});
104-
this.sendNotification({ method: 'exit' });
105-
106-
this.pendingNotificationMap.forEach(([res, timeout]) => {
107-
res(new Error('nxls stopped'));
108-
clearTimeout(timeout);
109-
});
110-
this.pendingRequestMap.forEach(([res, timeout], key) => {
111-
res({
112-
jsonrpc: '2.0',
113-
id: key,
114-
error: {
115-
code: -32000,
116-
message: 'nxls stopped',
117-
},
118-
});
119-
clearTimeout(timeout);
120-
});
121-
122-
this.readerDisposable?.dispose();
123-
this.readerErrorDisposable?.dispose();
124-
this.messageReader?.dispose();
125-
this.messageWriter?.dispose();
126-
127-
// make sure nothing can write to stdin anymore after we destroy the stream
128-
// this fixes an issue where a leftover 'Content Length' header would be written to the stdin
129-
// during the nxls shutdown sequence
130-
if (this.process?.stdin) {
131-
this.process.stdin.write = (chunk: any, cb: any) => {
132-
return true;
133-
};
102+
// Try graceful shutdown only if communication is healthy
103+
if (this.communicationHealthy) {
104+
try {
105+
await this.sendRequest(
106+
{
107+
method: 'shutdown',
108+
},
109+
0.5,
110+
); // Use shorter timeout for shutdown
111+
this.sendNotification({ method: 'exit' });
112+
} catch (e) {
113+
if (this.verbose) {
114+
console.log(
115+
'Graceful shutdown failed, proceeding with force cleanup',
116+
);
117+
}
118+
this.communicationHealthy = false;
119+
}
134120
}
135121

136-
this.process?.stdout?.destroy();
137-
this.process?.stderr?.destroy();
138-
this.process?.stdin?.destroy();
122+
// Cancel all pending operations
123+
this.cancelAllPendingOperations('nxls stopped');
139124

140-
this.process?.removeListener('exit', this.earlyExitListener);
125+
// Clean up all resources
126+
this.cleanupResources();
141127

142-
try {
143-
execSync(`npx nx@${version ?? defaultVersion} daemon --stop`, {
144-
cwd: this.cwd,
145-
});
146-
} catch (e) {
147-
console.error(e);
148-
}
128+
// Stop daemon
129+
this.stopDaemon(version);
149130

150-
if (this.process?.pid) {
151-
try {
152-
killGroup(this.process.pid);
153-
} catch (e) {
154-
console.log(`NXLS WRAPPER: ${e}`);
155-
}
156-
}
131+
// Force kill the process
132+
this.killProcess();
133+
134+
// Reset state
135+
this.communicationHealthy = true;
157136
}
158137

159138
async sendRequest(
@@ -216,6 +195,12 @@ export class NxlsWrapper {
216195
console.log(`waiting for ${method}`, this.pendingNotificationMap);
217196
}
218197
return await new Promise<any>((resolve, reject) => {
198+
// If communication is already unhealthy, reject immediately
199+
if (!this.communicationHealthy) {
200+
reject(new Error(`Cannot wait for ${method}: communication failed`));
201+
return;
202+
}
203+
219204
timeout = setTimeout(
220205
() => {
221206
this.pendingNotificationMap.delete(method);
@@ -236,53 +221,195 @@ export class NxlsWrapper {
236221
clearTimeout(timeout);
237222
}
238223

224+
private cancelAllPendingOperations(errorMessage = 'Communication failed') {
225+
// Cancel all pending notifications
226+
this.pendingNotificationMap.forEach(([resolve, timeout], method) => {
227+
resolve(new Error(`${errorMessage} while waiting for ${method}`));
228+
clearTimeout(timeout);
229+
});
230+
this.pendingNotificationMap.clear();
231+
232+
// Cancel all pending requests
233+
this.pendingRequestMap.forEach(([resolve, timeout], id) => {
234+
resolve({
235+
jsonrpc: '2.0',
236+
id,
237+
error: {
238+
code: -32000,
239+
message: errorMessage,
240+
},
241+
});
242+
clearTimeout(timeout);
243+
});
244+
this.pendingRequestMap.clear();
245+
}
246+
239247
setVerbose(verbose: boolean) {
240248
this.verbose = verbose;
241249
}
242250

243-
private listenToLSPMessages(messageReader: StreamMessageReader) {
244-
this.readerDisposable = messageReader.listen((message) => {
245-
if (
246-
isNotificationMessage(message) &&
247-
message.method === 'window/logMessage'
248-
) {
251+
isCommunicationHealthy(): boolean {
252+
return this.communicationHealthy;
253+
}
254+
255+
private cleanupResources() {
256+
// Clean up listeners first
257+
this.process?.removeListener('exit', this.earlyExitListener);
258+
259+
// Dispose of message handlers
260+
try {
261+
this.readerDisposable?.dispose();
262+
} catch (e) {
263+
// Ignore disposal errors
264+
}
265+
try {
266+
this.readerErrorDisposable?.dispose();
267+
} catch (e) {
268+
// Ignore disposal errors
269+
}
270+
271+
// Override stdin write before disposing streams
272+
if (this.process?.stdin && !this.process.stdin.destroyed) {
273+
this.process.stdin.write = (_chunk: any, cb: any) => {
274+
if (typeof cb === 'function') cb();
275+
return true;
276+
};
277+
}
278+
279+
// Dispose message reader/writer
280+
try {
281+
this.messageReader?.dispose();
282+
} catch (e) {
283+
// Ignore disposal errors
284+
}
285+
try {
286+
this.messageWriter?.dispose();
287+
} catch (e) {
288+
// Ignore disposal errors
289+
}
290+
291+
// Destroy streams
292+
try {
293+
this.process?.stdout?.destroy();
294+
this.process?.stderr?.destroy();
295+
this.process?.stdin?.destroy();
296+
} catch (e) {
297+
// Ignore stream destruction errors
298+
}
299+
}
300+
301+
private stopDaemon(version?: string, timeout = 5000) {
302+
try {
303+
execSync(`npx nx@${version ?? defaultVersion} daemon --stop`, {
304+
cwd: this.cwd,
305+
timeout,
306+
});
307+
} catch (e) {
308+
if (this.verbose) {
309+
console.error('Failed to stop daemon:', e);
310+
}
311+
}
312+
}
313+
314+
private killProcess() {
315+
if (this.process?.pid) {
316+
try {
317+
killGroup(this.process.pid);
318+
} catch (e) {
249319
if (this.verbose) {
250-
console.log((message.params as any)?.message);
320+
console.log(`NXLS WRAPPER: ${e}`);
251321
}
252-
return;
253-
}
254-
if (this.verbose) {
255-
console.log('received message', JSON.stringify(message, null, 2));
256322
}
323+
}
324+
}
325+
326+
async forceCleanup(version?: string) {
327+
if (this.verbose) {
328+
console.log('Forcing cleanup of NXLS process');
329+
}
330+
331+
this.communicationHealthy = false;
332+
333+
// Cancel all pending operations immediately
334+
this.cancelAllPendingOperations();
335+
336+
// Force kill the process first (different order than graceful shutdown)
337+
this.killProcess();
257338

258-
if (isResponseMessage(message) && typeof message.id === 'number') {
259-
const requestAndTimeout = this.pendingRequestMap.get(message.id);
260-
if (requestAndTimeout) {
261-
const [resolve, timeout] = requestAndTimeout;
262-
resolve(message);
263-
clearTimeout(timeout);
264-
this.pendingRequestMap.delete(message.id);
339+
// Stop daemon with shorter timeout
340+
this.stopDaemon(version, 3000);
341+
342+
// Clean up resources (but ignore errors during force cleanup)
343+
try {
344+
this.cleanupResources();
345+
} catch (e) {
346+
// Ignore cleanup errors during force cleanup
347+
}
348+
349+
this.communicationHealthy = true;
350+
}
351+
352+
private listenToLSPMessages(messageReader: StreamMessageReader) {
353+
this.readerDisposable = messageReader.listen((message) => {
354+
try {
355+
if (
356+
isNotificationMessage(message) &&
357+
message.method === 'window/logMessage'
358+
) {
359+
if (this.verbose) {
360+
console.log((message.params as any)?.message);
361+
}
362+
return;
265363
}
266-
} else if (isNotificationMessage(message)) {
267-
const method = message.method;
268364
if (this.verbose) {
269-
console.log('received notification', method);
270-
console.log('pending notifications', this.pendingNotificationMap);
271-
}
272-
const [resolve, timeout] =
273-
this.pendingNotificationMap.get(method) ?? [];
274-
if (resolve) {
275-
resolve(message.params);
276-
this.pendingNotificationMap.delete(method);
365+
console.log('received message', JSON.stringify(message, null, 2));
277366
}
278-
if (timeout) {
279-
clearTimeout(timeout);
367+
368+
if (isResponseMessage(message) && typeof message.id === 'number') {
369+
const requestAndTimeout = this.pendingRequestMap.get(message.id);
370+
if (requestAndTimeout) {
371+
const [resolve, timeout] = requestAndTimeout;
372+
resolve(message);
373+
clearTimeout(timeout);
374+
this.pendingRequestMap.delete(message.id);
375+
}
376+
} else if (isNotificationMessage(message)) {
377+
const method = message.method;
378+
if (this.verbose) {
379+
console.log('received notification', method);
380+
console.log('pending notifications', this.pendingNotificationMap);
381+
}
382+
const [resolve, timeout] =
383+
this.pendingNotificationMap.get(method) ?? [];
384+
if (resolve) {
385+
resolve(message.params);
386+
this.pendingNotificationMap.delete(method);
387+
}
388+
if (timeout) {
389+
clearTimeout(timeout);
390+
}
280391
}
392+
} catch (error) {
393+
console.error('Error processing message:', error);
394+
this.communicationHealthy = false;
395+
this.cancelAllPendingOperations();
281396
}
282397
});
283398

284399
this.readerErrorDisposable = messageReader.onError((error) => {
285400
console.error('ERROR: ', error);
401+
402+
// Check if this is a Content-Length error specifically
403+
const isContentLengthError = error.message?.includes('Content-Length');
404+
if (isContentLengthError) {
405+
console.error(
406+
'Content-Length error detected - communication corrupted',
407+
);
408+
}
409+
410+
this.communicationHealthy = false;
411+
// Cancel all pending operations when communication fails
412+
this.cancelAllPendingOperations();
286413
});
287414
}
288415
}
@@ -296,7 +423,3 @@ function isNotificationMessage(
296423
function isResponseMessage(message: Message): message is ResponseMessage {
297424
return 'result' in message || 'error' in message;
298425
}
299-
300-
function isRequestMessage(message: Message): message is RequestMessage {
301-
return !isNotificationMessage(message) && !isResponseMessage(message);
302-
}

0 commit comments

Comments
 (0)