From 9cb01a6dd9c6cbf428a1a44758abdf202f012d7c Mon Sep 17 00:00:00 2001 From: Andy Chen Date: Fri, 21 Mar 2025 16:26:12 -0700 Subject: [PATCH 1/3] Added warning to chatSession.sendMessage() and chatSession.sendMessageStream() so that when a promise is still processing, when a user sends a new message, they will get a notification to warn them of unexpected results --- package.json | 11 ++++++----- src/methods/chat-session.test.ts | 25 +++++++++++++++++++++++++ src/methods/chat-session.ts | 16 ++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 54266a7ca..2d47125f0 100644 --- a/package.json +++ b/package.json @@ -35,13 +35,13 @@ "build": "rollup -c && npm run api-report", "test": "npm run lint && npm run test:node:unit", "test:web:integration": "npm run build && npx web-test-runner", - "test:node:unit": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha \"src/**/*.test.ts\"", - "test:node:integration": "npm run build && TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha \"test-integration/node/**/*.test.ts\"", - "lint": "eslint -c .eslintrc.js '**/*.ts' --ignore-path './.gitignore'", + "test:node:unit": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"commonjs\\\"}\" mocha \"src/**/*.test.ts\"", + "test:node:integration": "npm run build && cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"commonjs\\\"}\" mocha \"test-integration/node/**/*.test.ts\"", + "lint": "eslint -c .eslintrc.js \"**/*.ts\" --ignore-path \"./.gitignore\"", "api-report": "api-extractor run -c api-extractor.json --local --verbose && api-extractor run -c api-extractor.server.json --local --verbose", "docs": "npm run build && npx api-documenter markdown -i ./temp/main -o ./docs/reference/main && npx api-documenter markdown -i ./temp/server -o ./docs/reference/server", - "format": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"nodenext\"}' npx ts-node scripts/run-format.ts", - "format:check": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"nodenext\"}' npx ts-node scripts/check-format.ts" + "format": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"nodenext\\\"}\" npx ts-node scripts/run-format.ts", + "format:check": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"nodenext\\\"}\" npx ts-node scripts/check-format.ts" }, "devDependencies": { "@changesets/cli": "^2.27.1", @@ -63,6 +63,7 @@ "chai": "^4.3.10", "chai-as-promised": "^7.1.1", "chai-deep-equal-ignore-undefined": "^1.1.1", + "cross-env": "^7.0.3", "eslint": "^8.52.0", "eslint-plugin-import": "^2.29.0", "eslint-plugin-unused-imports": "^3.0.0", diff --git a/src/methods/chat-session.test.ts b/src/methods/chat-session.test.ts index c09821030..5bcac3701 100644 --- a/src/methods/chat-session.test.ts +++ b/src/methods/chat-session.test.ts @@ -46,6 +46,15 @@ describe("ChatSession", () => { match.any, ); }); + it("sendMessage() should reset messageInProgress flag after resolving promise", async () => { + const mockResponse = getMockResponse( + "unary-success-basic-reply-short.json", + ); + stub(request, "makeModelRequest").resolves(mockResponse as Response); + const chatSession = new ChatSession("MY_API_KEY", "a-model"); + await chatSession.sendMessage("hello"); + expect(chatSession["_messageInProgress"]).to.be.false; + }); }); describe("sendMessageRecitationErrorNotAddingResponseToHistory()", () => { it("generateContent errors should be catchable", async () => { @@ -75,6 +84,22 @@ describe("ChatSession", () => { expect(consoleStub).to.not.be.called; clock.restore(); }); + it("sendMessageStream() should reset messageInProgress flag after resolving promise", async () => { + const consoleStub = stub(console, "error"); + const generateContentStreamStub = stub( + generateContentMethods, + "generateContentStream", + ).resolves(); + const chatSession = new ChatSession("MY_API_KEY", "a-model"); + await chatSession.sendMessageStream("hello"); + expect(generateContentStreamStub).to.be.calledWith( + "MY_API_KEY", + "a-model", + match.any, + ); + expect(consoleStub).to.not.be.called; + expect(chatSession["_messageInProgress"]).to.be.false; + }); it("downstream sendPromise errors should log but not throw", async () => { const clock = useFakeTimers(); const consoleStub = stub(console, "error"); diff --git a/src/methods/chat-session.ts b/src/methods/chat-session.ts index 4022e2e39..b4a536f0b 100644 --- a/src/methods/chat-session.ts +++ b/src/methods/chat-session.ts @@ -45,6 +45,7 @@ export class ChatSession { private _apiKey: string; private _history: Content[] = []; private _sendPromise: Promise = Promise.resolve(); + private _messageInProgress: boolean = false; constructor( apiKey: string, @@ -81,6 +82,12 @@ export class ChatSession { request: string | Array, requestOptions: SingleRequestOptions = {}, ): Promise { + if (this._messageInProgress) { + console.warn( + "sendMessage() was called while another message was in progress, this may lead to unexpected behavior. ", + ); + } + this._messageInProgress = true; await this._sendPromise; const newContent = formatNewContent(request); const generateContentRequest: GenerateContentRequest = { @@ -126,6 +133,7 @@ export class ChatSession { } } finalResult = result; + this._messageInProgress = false; }); await this._sendPromise; return finalResult; @@ -144,6 +152,12 @@ export class ChatSession { request: string | Array, requestOptions: SingleRequestOptions = {}, ): Promise { + if (this._messageInProgress) { + console.warn( + "sendMessage() was called while another message was in progress, this may lead to unexpected behavior. ", + ); + } + this._messageInProgress = true; await this._sendPromise; const newContent = formatNewContent(request); const generateContentRequest: GenerateContentRequest = { @@ -202,7 +216,9 @@ export class ChatSession { // downstream from streamPromise, so they should not throw. console.error(e); } + this._messageInProgress = false; }); + this._messageInProgress = false; return streamPromise; } } From 9abaa27ba8e057b83905be338e9c9ff3fd65c649 Mon Sep 17 00:00:00 2001 From: Andy Chen Date: Fri, 21 Mar 2025 16:49:40 -0700 Subject: [PATCH 2/3] made it so an error is throw when user tries to send new message before the first one is resolved --- src/methods/chat-session.test.ts | 2 +- src/methods/chat-session.ts | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/methods/chat-session.test.ts b/src/methods/chat-session.test.ts index 5bcac3701..ed36c67f7 100644 --- a/src/methods/chat-session.test.ts +++ b/src/methods/chat-session.test.ts @@ -91,7 +91,7 @@ describe("ChatSession", () => { "generateContentStream", ).resolves(); const chatSession = new ChatSession("MY_API_KEY", "a-model"); - await chatSession.sendMessageStream("hello"); + await expect(chatSession.sendMessageStream("hello")).to.be.fulfilled; expect(generateContentStreamStub).to.be.calledWith( "MY_API_KEY", "a-model", diff --git a/src/methods/chat-session.ts b/src/methods/chat-session.ts index b4a536f0b..9dfc9b78f 100644 --- a/src/methods/chat-session.ts +++ b/src/methods/chat-session.ts @@ -83,8 +83,8 @@ export class ChatSession { requestOptions: SingleRequestOptions = {}, ): Promise { if (this._messageInProgress) { - console.warn( - "sendMessage() was called while another message was in progress, this may lead to unexpected behavior. ", + throw new Error( + "sendMessage() was called while another message was in progress, this may lead to unexpected behavior.", ); } this._messageInProgress = true; @@ -133,6 +133,8 @@ export class ChatSession { } } finalResult = result; + }) + .finally(() => { this._messageInProgress = false; }); await this._sendPromise; @@ -153,8 +155,8 @@ export class ChatSession { requestOptions: SingleRequestOptions = {}, ): Promise { if (this._messageInProgress) { - console.warn( - "sendMessage() was called while another message was in progress, this may lead to unexpected behavior. ", + throw new Error( + "sendMessageStream() was called while another message was in progress, this may lead to unexpected behavior.", ); } this._messageInProgress = true; @@ -216,7 +218,6 @@ export class ChatSession { // downstream from streamPromise, so they should not throw. console.error(e); } - this._messageInProgress = false; }); this._messageInProgress = false; return streamPromise; From d0a290be4e6f2a037626ae1ab932da236663f44e Mon Sep 17 00:00:00 2001 From: Andy Chen Date: Fri, 21 Mar 2025 17:11:58 -0700 Subject: [PATCH 3/3] unstaged package.json change --- package.json | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 2d47125f0..54266a7ca 100644 --- a/package.json +++ b/package.json @@ -35,13 +35,13 @@ "build": "rollup -c && npm run api-report", "test": "npm run lint && npm run test:node:unit", "test:web:integration": "npm run build && npx web-test-runner", - "test:node:unit": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"commonjs\\\"}\" mocha \"src/**/*.test.ts\"", - "test:node:integration": "npm run build && cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"commonjs\\\"}\" mocha \"test-integration/node/**/*.test.ts\"", - "lint": "eslint -c .eslintrc.js \"**/*.ts\" --ignore-path \"./.gitignore\"", + "test:node:unit": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha \"src/**/*.test.ts\"", + "test:node:integration": "npm run build && TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha \"test-integration/node/**/*.test.ts\"", + "lint": "eslint -c .eslintrc.js '**/*.ts' --ignore-path './.gitignore'", "api-report": "api-extractor run -c api-extractor.json --local --verbose && api-extractor run -c api-extractor.server.json --local --verbose", "docs": "npm run build && npx api-documenter markdown -i ./temp/main -o ./docs/reference/main && npx api-documenter markdown -i ./temp/server -o ./docs/reference/server", - "format": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"nodenext\\\"}\" npx ts-node scripts/run-format.ts", - "format:check": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"nodenext\\\"}\" npx ts-node scripts/check-format.ts" + "format": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"nodenext\"}' npx ts-node scripts/run-format.ts", + "format:check": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"nodenext\"}' npx ts-node scripts/check-format.ts" }, "devDependencies": { "@changesets/cli": "^2.27.1", @@ -63,7 +63,6 @@ "chai": "^4.3.10", "chai-as-promised": "^7.1.1", "chai-deep-equal-ignore-undefined": "^1.1.1", - "cross-env": "^7.0.3", "eslint": "^8.52.0", "eslint-plugin-import": "^2.29.0", "eslint-plugin-unused-imports": "^3.0.0",