From cbadb8a8b8798932b92c34672f2b226d4337b54f Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Wed, 14 Dec 2022 16:30:09 -0500 Subject: [PATCH 1/2] add catch for when the playground worker server connection is disposed, and re-initialize the server --- playgrounds/out-of-memory-test.mongodb | 22 ++++++++ src/editors/playgroundController.ts | 45 +++++++++++----- src/language/languageServerController.ts | 8 +-- .../editors/playgroundController.test.ts | 52 ++++++++++++++++--- .../explorer/playgroundsExplorer.test.ts | 17 ++---- 5 files changed, 107 insertions(+), 37 deletions(-) create mode 100644 playgrounds/out-of-memory-test.mongodb diff --git a/playgrounds/out-of-memory-test.mongodb b/playgrounds/out-of-memory-test.mongodb new file mode 100644 index 000000000..fa3edcf58 --- /dev/null +++ b/playgrounds/out-of-memory-test.mongodb @@ -0,0 +1,22 @@ +/** + * This playground is used to test how the language server worker + * handles an out of memory error in mongosh's running of a playground. VSCODE-269 + */ + +use('test'); + +const mockDataArray = []; +for(let i = 0; i < 50000; i++) { + mockDataArray.push(Math.random() * 10000); +} + +const docs = []; +for(let i = 0; i < 10000000; i++) { + docs.push({ + mockData: [...mockDataArray], + a: 'test 123', + b: Math.ceil(Math.random() * 10000) + }); +} + +console.log('Should not show this message as the process should have run out of memory in the loop above.'); diff --git a/src/editors/playgroundController.ts b/src/editors/playgroundController.ts index 42d5bd8b8..06e6436d4 100644 --- a/src/editors/playgroundController.ts +++ b/src/editors/playgroundController.ts @@ -332,21 +332,40 @@ export default class PlaygroundController { this._statusView.showMessage('Getting results...'); - // Send a request to the language server to execute scripts from a playground. - const result: ShellExecuteAllResult = - await this._languageServerController.executeAll({ - codeToEvaluate, - connectionId, - }); + try { + // Send a request to the language server to execute scripts from a playground. + const result: ShellExecuteAllResult = + await this._languageServerController.executeAll({ + codeToEvaluate, + connectionId, + }); - this._statusView.hideMessage(); - this._telemetryService.trackPlaygroundCodeExecuted( - result, - this._isPartialRun, - result ? false : true - ); + this._statusView.hideMessage(); + this._telemetryService.trackPlaygroundCodeExecuted( + result, + this._isPartialRun, + result ? false : true + ); - return result; + return result; + } catch (err: any) { + // We re-initialize the language server when we encounter an error. + // This happens when the language server worker runs out of memory, can't be revitalized, and restarts. + if ( + err?.message?.includes( + 'Pending response rejected since connection got disposed' + ) + ) { + void vscode.window.showErrorMessage( + 'An error occurred when running the playground. This can occur when the playground runner runs out of memory.' + ); + + await this._languageServerController.startLanguageServer(); + void this._connectToServiceProvider(); + } + + throw err; + } } _getAllText(): string { diff --git a/src/language/languageServerController.ts b/src/language/languageServerController.ts index a8ff667e5..c079fa0b2 100644 --- a/src/language/languageServerController.ts +++ b/src/language/languageServerController.ts @@ -89,10 +89,12 @@ export default class LanguageServerController { async startLanguageServer(): Promise { // Push the disposable client to the context's subscriptions so that the - // client can be deactivated on extension deactivation - this._context.subscriptions.push(this._client); + // client can be deactivated on extension deactivation. + if (!this._context.subscriptions.includes(this._client)) { + this._context.subscriptions.push(this._client); + } - // Subscribe on notifications from the server when the client is ready + // Subscribe on notifications from the server when the client is ready. await this._client.sendRequest( ServerCommands.SET_EXTENSION_PATH, this._context.extensionPath diff --git a/src/test/suite/editors/playgroundController.test.ts b/src/test/suite/editors/playgroundController.test.ts index 2b5b5c7ef..2a1362d80 100644 --- a/src/test/suite/editors/playgroundController.test.ts +++ b/src/test/suite/editors/playgroundController.test.ts @@ -406,6 +406,45 @@ suite('Playground Controller Test Suite', function () { }); }); + test('it shows an error message and restarts, and connects the language server when an error occurs in executeAll (out of memory can cause this)', async () => { + sinon + .stub(mockLanguageServerController, 'executeAll') + .rejects( + new Error('Pending response rejected since connection got disposed') + ); + + const stubStartLanguageServer = sinon + .stub(mockLanguageServerController, 'startLanguageServer') + .resolves(); + + const stubConnectToServiceProvider = sinon + .stub(testPlaygroundController, '_connectToServiceProvider') + .resolves(); + + const stubVSCodeErrorMessage = sinon + .stub(vscode.window, 'showErrorMessage') + .resolves(undefined); + + try { + await testPlaygroundController._evaluate('console.log("test");'); + + // It should have thrown in the above evaluation. + expect(true).to.equal(false); + } catch (err: any) { + expect(err.message).to.equal( + 'Pending response rejected since connection got disposed' + ); + } + + expect(stubVSCodeErrorMessage.calledOnce).to.equal(true); + expect(stubVSCodeErrorMessage.firstCall.args[0]).to.equal( + 'An error occurred when running the playground. This can occur when the playground runner runs out of memory.' + ); + + expect(stubStartLanguageServer.calledOnce).to.equal(true); + expect(stubConnectToServiceProvider.calledOnce).to.equal(true); + }); + test('playground controller loads the active editor on start', () => { sandbox.replaceGetter( vscode.window, @@ -458,12 +497,9 @@ suite('Playground Controller Test Suite', function () { document: { getText: () => textFromEditor }, } as vscode.TextEditor; - const fakeVscodeErrorMessage: any = sinon.fake(); - sinon.replace( - vscode.window, - 'showErrorMessage', - fakeVscodeErrorMessage - ); + const fakeVscodeErrorMessage = sinon + .stub(vscode.window, 'showErrorMessage') + .resolves(undefined); playgroundControllerTest._selectedText = '{ name: qwerty }'; playgroundControllerTest._codeActionProvider.selection = selection; @@ -474,7 +510,9 @@ suite('Playground Controller Test Suite', function () { const expectedMessage = "Unable to export to csharp language: Symbol 'qwerty' is undefined"; - expect(fakeVscodeErrorMessage.firstArg).to.be.equal(expectedMessage); + expect(fakeVscodeErrorMessage.firstCall.args[0]).to.equal( + expectedMessage + ); }); }); }); diff --git a/src/test/suite/explorer/playgroundsExplorer.test.ts b/src/test/suite/explorer/playgroundsExplorer.test.ts index 7fe30ae1f..0994fa9a7 100644 --- a/src/test/suite/explorer/playgroundsExplorer.test.ts +++ b/src/test/suite/explorer/playgroundsExplorer.test.ts @@ -54,21 +54,13 @@ suite('Playgrounds Controller Test Suite', function () { try { const children = await treeController.getPlaygrounds(rootUri); - assert.strictEqual( - Object.keys(children).length, - 5, - `Tree playgrounds should have 5 child, found ${children.length}` - ); + assert.strictEqual(Object.keys(children).length, 6); const playgrounds = Object.values(children).filter( (item: any) => item.label && item.label.split('.').pop() === 'mongodb' ); - assert.strictEqual( - Object.keys(playgrounds).length, - 5, - `Tree playgrounds should have 5 playgrounds with mongodb extension, found ${children.length}` - ); + assert.strictEqual(Object.keys(playgrounds).length, 6); } catch (error) { assert(false, error as Error); } @@ -92,10 +84,7 @@ suite('Playgrounds Controller Test Suite', function () { const treeController = new PlaygroundsTree(); const children = await treeController.getPlaygrounds(rootUri); - assert( - Object.keys(children).length === 3, - `Tree playgrounds should have 3 child, found ${children.length}` - ); + assert.strictEqual(Object.keys(children).length, 3); } catch (error) { assert(false, error as Error); } From 89fc28e8d27ad69250fd75571f3b86770c90ab5e Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Thu, 15 Dec 2022 10:03:17 -0500 Subject: [PATCH 2/2] use error code instead of error message --- src/editors/playgroundController.ts | 6 +----- src/test/suite/editors/playgroundController.test.ts | 9 ++++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/editors/playgroundController.ts b/src/editors/playgroundController.ts index 06e6436d4..f19bfd41b 100644 --- a/src/editors/playgroundController.ts +++ b/src/editors/playgroundController.ts @@ -351,11 +351,7 @@ export default class PlaygroundController { } catch (err: any) { // We re-initialize the language server when we encounter an error. // This happens when the language server worker runs out of memory, can't be revitalized, and restarts. - if ( - err?.message?.includes( - 'Pending response rejected since connection got disposed' - ) - ) { + if (err?.code === -32097) { void vscode.window.showErrorMessage( 'An error occurred when running the playground. This can occur when the playground runner runs out of memory.' ); diff --git a/src/test/suite/editors/playgroundController.test.ts b/src/test/suite/editors/playgroundController.test.ts index 2a1362d80..760b28e77 100644 --- a/src/test/suite/editors/playgroundController.test.ts +++ b/src/test/suite/editors/playgroundController.test.ts @@ -407,11 +407,13 @@ suite('Playground Controller Test Suite', function () { }); test('it shows an error message and restarts, and connects the language server when an error occurs in executeAll (out of memory can cause this)', async () => { + const mockConnectionDisposedError = new Error( + 'Pending response rejected since connection got disposed' + ); + (mockConnectionDisposedError as any).code = -32097; sinon .stub(mockLanguageServerController, 'executeAll') - .rejects( - new Error('Pending response rejected since connection got disposed') - ); + .rejects(mockConnectionDisposedError); const stubStartLanguageServer = sinon .stub(mockLanguageServerController, 'startLanguageServer') @@ -434,6 +436,7 @@ suite('Playground Controller Test Suite', function () { expect(err.message).to.equal( 'Pending response rejected since connection got disposed' ); + expect(err.code).to.equal(-32097); } expect(stubVSCodeErrorMessage.calledOnce).to.equal(true);