From 057edbab16852247e3df1b176ed2d20f66f754fb Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 5 Feb 2025 12:26:25 +0100 Subject: [PATCH] add sync vs async test case for error'ing `provideLanguageModelResponse` calls (#239676) https://github.com/microsoft/vscode/issues/235322 --- .../src/singlefolder-tests/lm.test.ts | 40 +++++++++++++---- .../api/browser/mainThreadLanguageModels.ts | 8 +++- .../api/common/extHostLanguageModels.ts | 44 ++++++++++--------- 3 files changed, 63 insertions(+), 29 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/lm.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/lm.test.ts index cc995c08497..97875753f88 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/lm.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/lm.test.ts @@ -139,11 +139,11 @@ suite('lm', function () { } }); - test('LanguageModelError instance is not thrown to extensions#235322', async function () { + test('LanguageModelError instance is not thrown to extensions#235322 (SYNC)', async function () { disposables.push(vscode.lm.registerChatModelProvider('test-lm', { - async provideLanguageModelResponse(_messages, _options, _extensionId, _progress, _token) { - throw vscode.LanguageModelError.Blocked('You have been blocked'); + provideLanguageModelResponse(_messages, _options, _extensionId, _progress, _token) { + throw vscode.LanguageModelError.Blocked('You have been blocked SYNC'); }, async provideTokenCount(_text, _token) { return 1; @@ -153,17 +153,41 @@ suite('lm', function () { const models = await vscode.lm.selectChatModels({ id: 'test-lm' }); assert.strictEqual(models.length, 1); - let output = ''; - try { - const response = await models[0].sendRequest([vscode.LanguageModelChatMessage.User('Hello')]); - assert.ok(response); + await models[0].sendRequest([vscode.LanguageModelChatMessage.User('Hello')]); + assert.ok(false, 'EXPECTED error'); + } catch (error) { + assert.ok(error instanceof vscode.LanguageModelError); + assert.strictEqual(error.message, 'You have been blocked SYNC'); + } + }); + + test('LanguageModelError instance is not thrown to extensions#235322 (ASYNC)', async function () { + + disposables.push(vscode.lm.registerChatModelProvider('test-lm', { + async provideLanguageModelResponse(_messages, _options, _extensionId, _progress, _token) { + throw vscode.LanguageModelError.Blocked('You have been blocked ASYNC'); + }, + async provideTokenCount(_text, _token) { + return 1; + } + }, testProviderOptions)); + + const models = await vscode.lm.selectChatModels({ id: 'test-lm' }); + assert.strictEqual(models.length, 1); + + + const response = await models[0].sendRequest([vscode.LanguageModelChatMessage.User('Hello')]); + assert.ok(response); + + let output = ''; + try { for await (const thing of response.text) { output += thing; } } catch (error) { assert.ok(error instanceof vscode.LanguageModelError); - assert.strictEqual(error.message, 'You have been blocked'); + assert.strictEqual(error.message, 'You have been blocked ASYNC'); } assert.strictEqual(output, ''); }); diff --git a/src/vs/workbench/api/browser/mainThreadLanguageModels.ts b/src/vs/workbench/api/browser/mainThreadLanguageModels.ts index 1b1cb20d72b..693eab881f2 100644 --- a/src/vs/workbench/api/browser/mainThreadLanguageModels.ts +++ b/src/vs/workbench/api/browser/mainThreadLanguageModels.ts @@ -123,7 +123,13 @@ export class MainThreadLanguageModels implements MainThreadLanguageModelsShape { async $tryStartChatRequest(extension: ExtensionIdentifier, providerId: string, requestId: number, messages: IChatMessage[], options: {}, token: CancellationToken): Promise { this._logService.trace('[CHAT] request STARTED', extension.value, requestId); - const response = await this._chatProviderService.sendChatRequest(providerId, extension, messages, options, token); + let response: ILanguageModelChatResponse; + try { + response = await this._chatProviderService.sendChatRequest(providerId, extension, messages, options, token); + } catch (err) { + this._logService.error('[CHAT] request FAILED', extension.value, requestId, err); + throw err; + } // !!! IMPORTANT !!! // This method must return before the response is done (has streamed all parts) diff --git a/src/vs/workbench/api/common/extHostLanguageModels.ts b/src/vs/workbench/api/common/extHostLanguageModels.ts index 7ae740ff9fa..ff74aa62a75 100644 --- a/src/vs/workbench/api/common/extHostLanguageModels.ts +++ b/src/vs/workbench/api/common/extHostLanguageModels.ts @@ -220,30 +220,34 @@ export class ExtHostLanguageModels implements ExtHostLanguageModelsShape { this._proxy.$reportResponsePart(requestId, { index: fragment.index, part }); }); - let p: Promise; + let value: any; - if (data.provider.provideLanguageModelResponse2) { + try { + if (data.provider.provideLanguageModelResponse2) { + value = data.provider.provideLanguageModelResponse2( + messages.map(typeConvert.LanguageModelChatMessage.to), + options, + ExtensionIdentifier.toKey(from), + progress, + token + ); - p = Promise.resolve(data.provider.provideLanguageModelResponse2( - messages.map(typeConvert.LanguageModelChatMessage.to), - options, - ExtensionIdentifier.toKey(from), - progress, - token - )); + } else { + value = data.provider.provideLanguageModelResponse( + messages.map(typeConvert.LanguageModelChatMessage.to), + options, + ExtensionIdentifier.toKey(from), + progress, + token + ); + } - } else { - - p = Promise.resolve(data.provider.provideLanguageModelResponse( - messages.map(typeConvert.LanguageModelChatMessage.to), - options, - ExtensionIdentifier.toKey(from), - progress, - token - )); + } catch (err) { + // synchronously failed + throw err; } - p.then(() => { + Promise.resolve(value).then(() => { this._proxy.$reportResponseDone(requestId, undefined); }, err => { this._proxy.$reportResponseDone(requestId, transformErrorForSerialization(err)); @@ -391,7 +395,7 @@ export class ExtHostLanguageModels implements ExtHostLanguageModelsShape { // error'ing here means that the request could NOT be started/made, e.g. wrong model, no access, etc, but // later the response can fail as well. Those failures are communicated via the stream-object this._pendingRequest.delete(requestId); - throw error; + throw extHostTypes.LanguageModelError.tryDeserialize(error) ?? error; } return res.apiObject;