From 482312dcfe2cdd4589ffa06080087361ef467f37 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:50:48 -0700 Subject: [PATCH 1/2] Skip workspace diagnostic reports that fail to convert The final workspace/diagnostic response converted its reports without per-item error handling, unlike the partial-result handler. A single report that failed to convert (e.g. a diagnostic with an empty message, which VS Code rejects with "message must be set") rejected the whole pull, so the valid reports were dropped. Since the same payload fails on every pull, workspaceErrorCounter passed its limit after 6 attempts and workspace pulls stopped for the session. Log and skip the failing report, as the partial-result path does. The progress handler leak test now expects the pull to resolve, and a new test covers keeping the valid reports. --- client-node-tests/src/integration.test.ts | 40 +++++++++++++++++++---- client/src/common/diagnostic.ts | 8 +++-- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index 1672efb37..bbcd88c58 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -1679,7 +1679,7 @@ suite('Client integration', () => { * settled. Only registrations made synchronously by this call are counted, so * the client's background workspace pull doesn't interfere. */ - async function workspacePullProgressHandlers(token: vscode.CancellationToken, expectRejection?: RegExp): Promise<{ registered: (string | number)[]; active: (string | number)[] }> { + async function workspacePullProgressHandlers(token: vscode.CancellationToken): Promise<{ registered: (string | number)[]; active: (string | number)[] }> { const provider = client.getFeature(lsclient.DocumentDiagnosticRequest.method)?.getProvider(document); isDefined(provider); isDefined(provider.diagnostics.provideWorkspaceDiagnostics); @@ -1695,11 +1695,7 @@ suite('Client integration', () => { } finally { onProgress.restore(); } - if (expectRejection !== undefined) { - await assert.rejects(pull, expectRejection); - } else { - await pull; - } + await pull; return { registered, active: registered.filter(item => progressHandlers.has(item)) }; } @@ -1750,7 +1746,7 @@ suite('Client integration', () => { }); let handlers: { registered: (string | number)[]; active: (string | number)[] }; try { - handlers = await workspacePullProgressHandlers(source.token, /conversion failed/); + handlers = await workspacePullProgressHandlers(source.token); } finally { stub.restore(); source.dispose(); @@ -1759,6 +1755,36 @@ suite('Client integration', () => { assert.strictEqual(handlers.active.length, 0, 'expected the progress handler to be disposed'); }); + test('Workspace diagnostic pull skips a report that fails to convert and keeps the rest', async () => { + const provider = client.getFeature(lsclient.DocumentDiagnosticRequest.method)?.getProvider(document); + isDefined(provider); + isDefined(provider.diagnostics.provideWorkspaceDiagnostics); + const source = new vscode.CancellationTokenSource(); + const sendRequest = client.sendRequest.bind(client) as (...args: any[]) => Promise; + // Append a report whose empty message makes VS Code's Diagnostic constructor throw. + const stub = sinon.stub(client, 'sendRequest').callsFake(async (...args: any[]) => { + const result = await sendRequest(...args); + if (args[2] === source.token) { + result.items.push({ kind: 'full', uri: 'file:///bad.txt', items: [{ range: { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } }, message: '' }] }); + } + return result; + }); + const errorLog = sinon.stub(client, 'error'); + const chunks: (vsdiag.WorkspaceDiagnosticReportPartialResult | null)[] = []; + try { + await provider.diagnostics.provideWorkspaceDiagnostics([], source.token, (chunk) => chunks.push(chunk)); + } finally { + stub.restore(); + errorLog.restore(); + source.dispose(); + } + assert.strictEqual(chunks.length, 1, 'expected the final report to be delivered'); + const items = chunks[0]?.items ?? []; + assert.strictEqual(items.length, 1, 'expected the valid report to be kept'); + assert.notStrictEqual(items[0].uri.toString(), 'file:///bad.txt', 'expected the bad report to be skipped'); + assert.ok(errorLog.calledOnceWith('Converting workspace diagnostics failed.'), 'expected the conversion failure to be logged'); + }); + test('Document diagnostic pull after quick reopen', async () => { await vscode.window.showTextDocument(document); let initialPullFinished: (() => void) | undefined; diff --git a/client/src/common/diagnostic.ts b/client/src/common/diagnostic.ts index 92429105b..31ea6b783 100644 --- a/client/src/common/diagnostic.ts +++ b/client/src/common/diagnostic.ts @@ -644,14 +644,18 @@ class DiagnosticRequestor implements Disposable { items: [] }; for (const item of result.items) { - converted.items.push(await convertReport(item)); + try { + converted.items.push(await convertReport(item)); + } catch (error) { + this.client.error(`Converting workspace diagnostics failed.`, error); + } } resultReporter(converted); return { items: [] }; }, (error) => { return this.client.handleFailedRequest(DocumentDiagnosticRequest.type, token, error, { items: [] }); }).finally(() => { - // Runs on success, cancellation, request failure, and a convertReport throw. + // Runs on success, cancellation, and request failure. disposable.dispose(); }); }; From 87a4c85ed6106e9cbf5996de44a85cf6eaf0b944 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:06:34 -0700 Subject: [PATCH 2/2] fixup! Skip workspace diagnostic reports that fail to convert --- client-node-tests/src/integration.test.ts | 2 +- client/src/common/diagnostic.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index bbcd88c58..d4cb9662e 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -1782,7 +1782,7 @@ suite('Client integration', () => { const items = chunks[0]?.items ?? []; assert.strictEqual(items.length, 1, 'expected the valid report to be kept'); assert.notStrictEqual(items[0].uri.toString(), 'file:///bad.txt', 'expected the bad report to be skipped'); - assert.ok(errorLog.calledOnceWith('Converting workspace diagnostics failed.'), 'expected the conversion failure to be logged'); + assert.ok(errorLog.calledOnceWithExactly('Converting workspace diagnostics failed.', sinon.match.any, false), 'expected the conversion failure to be logged without a notification'); }); test('Document diagnostic pull after quick reopen', async () => { diff --git a/client/src/common/diagnostic.ts b/client/src/common/diagnostic.ts index 31ea6b783..53883cf5f 100644 --- a/client/src/common/diagnostic.ts +++ b/client/src/common/diagnostic.ts @@ -626,7 +626,7 @@ class DiagnosticRequestor implements Disposable { try { converted.items.push(await convertReport(item)); } catch (error) { - this.client.error(`Converting workspace diagnostics failed.`, error); + this.client.error(`Converting workspace diagnostics failed.`, error, false); } } resultReporter(converted); @@ -647,7 +647,7 @@ class DiagnosticRequestor implements Disposable { try { converted.items.push(await convertReport(item)); } catch (error) { - this.client.error(`Converting workspace diagnostics failed.`, error); + this.client.error(`Converting workspace diagnostics failed.`, error, false); } } resultReporter(converted);