From 3687a7eabefac3529d064366339b395f00ebddc3 Mon Sep 17 00:00:00 2001 From: Teresa Hoang Date: Tue, 8 Aug 2023 14:48:55 -0700 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=9B=20Fix=20connection=20errors=20?= =?UTF-8?q?in=20useChat=20and=20appSlice,=20functional=20updates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitignore | 5 +- webapi/Controllers/ChatController.cs | 4 +- webapp/src/Constants.ts | 1 + webapp/src/checkEnv.ts | 3 +- webapp/src/components/views/BackendProbe.tsx | 8 ++-- webapp/src/libs/hooks/useChat.ts | 34 +++++++------- webapp/src/redux/features/app/AppState.ts | 1 + webapp/src/redux/features/app/appSlice.ts | 46 +++++++++++++++++-- .../message-relay/signalRMiddleware.ts | 21 +++++++-- 9 files changed, 93 insertions(+), 30 deletions(-) diff --git a/.gitignore b/.gitignore index b6e361a29..f09d4d638 100644 --- a/.gitignore +++ b/.gitignore @@ -480,4 +480,7 @@ webapp/build/ webapp/node_modules/ # Custom plugin files used in webapp for testing -webapp/public/.well-known* \ No newline at end of file +webapp/public/.well-known* + +# Auto-generated solution file from Visual Studio +webapi/CopilotChatWebApi.sln \ No newline at end of file diff --git a/webapi/Controllers/ChatController.cs b/webapi/Controllers/ChatController.cs index 8b3edc1be..85e9e4769 100644 --- a/webapi/Controllers/ChatController.cs +++ b/webapi/Controllers/ChatController.cs @@ -184,7 +184,7 @@ private async Task RegisterPlannerSkillsAsync(CopilotChatPlanner planner, Dictio BearerAuthenticationProvider authenticationProvider = new(() => Task.FromResult(GithubAuthHeader)); await planner.Kernel.ImportAIPluginAsync( skillName: "GitHubPlugin", - filePath: Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "CopilotChat", "Skills", "OpenApiPlugins/GitHubPlugin/openapi.json"), + filePath: Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "Skills", "OpenApiPlugins/GitHubPlugin/openapi.json"), new OpenApiSkillExecutionParameters { AuthCallback = authenticationProvider.AuthenticateRequestAsync, @@ -200,7 +200,7 @@ await planner.Kernel.ImportAIPluginAsync( await planner.Kernel.ImportAIPluginAsync( skillName: "JiraPlugin", - filePath: Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "CopilotChat", "Skills", "OpenApiPlugins/JiraPlugin/openapi.json"), + filePath: Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "Skills", "OpenApiPlugins/JiraPlugin/openapi.json"), new OpenApiSkillExecutionParameters { AuthCallback = authenticationProvider.AuthenticateRequestAsync, diff --git a/webapp/src/Constants.ts b/webapp/src/Constants.ts index 0e8c70794..d3b709ce8 100644 --- a/webapp/src/Constants.ts +++ b/webapp/src/Constants.ts @@ -4,6 +4,7 @@ export const Constants = { app: { name: 'Copilot', updateCheckIntervalSeconds: 60 * 5, + CONNECTION_ALERT_ID: 'connection-alert', }, msal: { method: 'redirect', // 'redirect' | 'popup' diff --git a/webapp/src/checkEnv.ts b/webapp/src/checkEnv.ts index afa5a8afd..2a2f2fbb3 100644 --- a/webapp/src/checkEnv.ts +++ b/webapp/src/checkEnv.ts @@ -1,7 +1,8 @@ +// Checks if all required environment variables are defined +// Returns an array of missing variables export const getMissingEnvVariables = () => { // Should be aligned with variables defined in .env.example const envVariables = ['REACT_APP_BACKEND_URI', 'REACT_APP_AAD_AUTHORITY', 'REACT_APP_AAD_CLIENT_ID']; - const missingVariables = []; for (const variable of envVariables) { diff --git a/webapp/src/components/views/BackendProbe.tsx b/webapp/src/components/views/BackendProbe.tsx index 1a27f2018..fdb5b30a2 100644 --- a/webapp/src/components/views/BackendProbe.tsx +++ b/webapp/src/components/views/BackendProbe.tsx @@ -20,7 +20,9 @@ const BackendProbe: FC = ({ uri, onBackendFound }) => { } }; - void fetchAsync(); + void fetchAsync().catch(() => { + // Ignore - this page is just a probe, so we don't need to show any errors if backend is not found + }); }, 3000); return () => { @@ -33,8 +35,8 @@ const BackendProbe: FC = ({ uri, onBackendFound }) => { Looking for your backend - This sample expects to find a Semantic Kernel service from{' '} - webapi/ running at {uri} + This sample expects to find a Semantic Kernel service from webapi/ running at{' '} + {uri} Run your Semantic Kernel service locally using Visual Studio, Visual Studio Code or by typing the diff --git a/webapp/src/libs/hooks/useChat.ts b/webapp/src/libs/hooks/useChat.ts index e4751107f..095a5b2f2 100644 --- a/webapp/src/libs/hooks/useChat.ts +++ b/webapp/src/libs/hooks/useChat.ts @@ -72,10 +72,9 @@ export const useChat = () => { const createChat = async () => { const chatTitle = `Copilot @ ${new Date().toLocaleString()}`; - const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); try { await chatService - .createChatAsync(userId, chatTitle, accessToken) + .createChatAsync(userId, chatTitle, await AuthHelper.getSKaaSAccessToken(instance, inProgress)) .then((result: ICreateChatSessionResponse) => { const newChat: ChatState = { id: result.chatSession.id, @@ -148,8 +147,8 @@ export const useChat = () => { }; const loadChats = async () => { - const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); try { + const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); const chatSessions = await chatService.getAllChatsAsync(userId, accessToken); if (chatSessions.length > 0) { @@ -201,10 +200,9 @@ export const useChat = () => { }; const uploadBot = async (bot: Bot) => { - const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); - botService - .uploadAsync(bot, userId, accessToken) - .then(async (chatSession: IChatSession) => { + try { + const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); + await botService.uploadAsync(bot, userId, accessToken).then(async (chatSession: IChatSession) => { const chatMessages = await chatService.getChatMessagesAsync(chatSession.id, 0, 100, accessToken); const newChat = { @@ -217,11 +215,11 @@ export const useChat = () => { }; dispatch(addConversation(newChat)); - }) - .catch((e: any) => { - const errorMessage = `Unable to upload the bot. Details: ${getErrorDetails(e)}`; - dispatch(addAlert({ message: errorMessage, type: AlertType.Error })); }); + } catch (e: any) { + const errorMessage = `Unable to upload the bot. Details: ${getErrorDetails(e)}`; + dispatch(addAlert({ message: errorMessage, type: AlertType.Error })); + } }; const getBotProfilePicture = (index: number): string => { @@ -282,8 +280,8 @@ export const useChat = () => { }; const joinChat = async (chatId: string) => { - const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); try { + const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); await chatService.joinChatAsync(userId, chatId, accessToken).then(async (result: IChatSession) => { // Get chat messages const chatMessages = await chatService.getChatMessagesAsync(result.id, 0, 100, accessToken); @@ -315,9 +313,14 @@ export const useChat = () => { }; const editChat = async (chatId: string, title: string, syetemDescription: string, memoryBalance: number) => { - const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); try { - await chatService.editChatAsync(chatId, title, syetemDescription, memoryBalance, accessToken); + await chatService.editChatAsync( + chatId, + title, + syetemDescription, + memoryBalance, + await AuthHelper.getSKaaSAccessToken(instance, inProgress), + ); } catch (e: any) { const errorMessage = `Error editing chat ${chatId}. Details: ${getErrorDetails(e)}`; dispatch(addAlert({ message: errorMessage, type: AlertType.Error })); @@ -325,9 +328,8 @@ export const useChat = () => { }; const getServiceOptions = async () => { - const accessToken = await AuthHelper.getSKaaSAccessToken(instance, inProgress); try { - return await chatService.getServiceOptionsAsync(accessToken); + return await chatService.getServiceOptionsAsync(await AuthHelper.getSKaaSAccessToken(instance, inProgress)); } catch (e: any) { const errorMessage = `Error getting service options. Details: ${getErrorDetails(e)}`; dispatch(addAlert({ message: errorMessage, type: AlertType.Error })); diff --git a/webapp/src/redux/features/app/AppState.ts b/webapp/src/redux/features/app/AppState.ts index a1664f69e..11cd7ecb0 100644 --- a/webapp/src/redux/features/app/AppState.ts +++ b/webapp/src/redux/features/app/AppState.ts @@ -13,6 +13,7 @@ export interface ActiveUserInfo { export interface Alert { message: string; type: AlertType; + id?: string; } interface Feature { diff --git a/webapp/src/redux/features/app/appSlice.ts b/webapp/src/redux/features/app/appSlice.ts index ff95e23b3..1d4ef00f2 100644 --- a/webapp/src/redux/features/app/appSlice.ts +++ b/webapp/src/redux/features/app/appSlice.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. import { createSlice, PayloadAction } from '@reduxjs/toolkit'; +import { Constants } from '../../../Constants'; import { ServiceOptions } from '../../../libs/models/ServiceOptions'; import { TokenUsage } from '../../../libs/models/TokenUsage'; import { ActiveUserInfo, Alert, AppState, FeatureKeys, initialState } from './AppState'; @@ -13,10 +14,14 @@ export const appSlice = createSlice({ state.alerts = action.payload; }, addAlert: (state: AppState, action: PayloadAction) => { - if (state.alerts.length === 3) { - state.alerts.shift(); + if ( + action.payload.id == Constants.app.CONNECTION_ALERT_ID || + isServerConnectionError(action.payload.message) + ) { + updateConnectionStatus(state, action.payload); + } else { + addNewAlert(state.alerts, action.payload); } - state.alerts.push(action.payload); }, removeAlert: (state: AppState, action: PayloadAction) => { state.alerts.splice(action.payload, 1); @@ -89,3 +94,38 @@ const getTotalTokenUsage = (previousSum?: number, current?: number) => { return previousSum + current; }; + +const isServerConnectionError = (message: string) => { + return ( + message.includes(`Cannot send data if the connection is not in the 'Connected' State.`) || + message.includes(`Server timeout elapsed without receiving a message from the server.`) + ); +}; + +const addNewAlert = (alerts: Alert[], newAlert: Alert) => { + if (alerts.length === 3) { + alerts.shift(); + } + + alerts.push(newAlert); +}; + +const updateConnectionStatus = (state: AppState, statusUpdate: Alert) => { + if (isServerConnectionError(statusUpdate.message)) { + statusUpdate.message = + // Constant message so alert UI doesn't feel glitchy on every connection error from SignalR + 'Cannot send data due to lost connection or server timeout. Try refreshing this page to restart the connection.'; + } + + // There should only ever be one connection alert at a time, + // so we tag the alert with a unique ID so we can remove if needed + statusUpdate.id ??= Constants.app.CONNECTION_ALERT_ID; + + // Remove the existing connection alert if it exists + const connectionAlertIndex = state.alerts.findIndex((alert) => alert.id === Constants.app.CONNECTION_ALERT_ID); + if (connectionAlertIndex !== -1) { + state.alerts.splice(connectionAlertIndex, 1); + } + + addNewAlert(state.alerts, statusUpdate); +}; diff --git a/webapp/src/redux/features/message-relay/signalRMiddleware.ts b/webapp/src/redux/features/message-relay/signalRMiddleware.ts index 8ffc237ef..6beaac3fd 100644 --- a/webapp/src/redux/features/message-relay/signalRMiddleware.ts +++ b/webapp/src/redux/features/message-relay/signalRMiddleware.ts @@ -2,6 +2,7 @@ import * as signalR from '@microsoft/signalr'; import { AnyAction, Dispatch } from '@reduxjs/toolkit'; +import { Constants } from '../../../Constants'; import { AlertType } from '../../../libs/models/AlertType'; import { IChatUser } from '../../../libs/models/ChatUser'; import { PlanState } from '../../../libs/models/Plan'; @@ -65,7 +66,13 @@ const registerCommonSignalConnectionEvents = (store: Store) => { hubConnection.onclose((error) => { if (hubConnection.state === signalR.HubConnectionState.Disconnected) { const errorMessage = 'Connection closed due to error. Try refreshing this page to restart the connection'; - store.dispatch(addAlert({ message: String(errorMessage), type: AlertType.Error })); + store.dispatch( + addAlert({ + message: String(errorMessage), + type: AlertType.Error, + id: Constants.app.CONNECTION_ALERT_ID, + }), + ); console.log(errorMessage, error); } }); @@ -73,15 +80,21 @@ const registerCommonSignalConnectionEvents = (store: Store) => { hubConnection.onreconnecting((error) => { if (hubConnection.state === signalR.HubConnectionState.Reconnecting) { const errorMessage = 'Connection lost due to error. Reconnecting...'; - store.dispatch(addAlert({ message: String(errorMessage), type: AlertType.Info })); + store.dispatch( + addAlert({ + message: String(errorMessage), + type: AlertType.Info, + id: Constants.app.CONNECTION_ALERT_ID, + }), + ); console.log(errorMessage, error); } }); hubConnection.onreconnected((connectionId = '') => { if (hubConnection.state === signalR.HubConnectionState.Connected) { - const message = 'Connection reestablished.'; - store.dispatch(addAlert({ message, type: AlertType.Success })); + const message = 'Connection reestablished. Please refresh the page to ensure you have the latest data.'; + store.dispatch(addAlert({ message, type: AlertType.Success, id: Constants.app.CONNECTION_ALERT_ID })); console.log(message + ` Connected with connectionId ${connectionId}`); } }); From ed3a94c9a1acd3881fa78a15ac794f3c72ec0616 Mon Sep 17 00:00:00 2001 From: Teresa Hoang Date: Tue, 8 Aug 2023 16:42:18 -0700 Subject: [PATCH 2/3] Addressing comments --- webapp/src/checkEnv.ts | 6 ++++-- webapp/src/components/token-usage/TokenUsageGraph.tsx | 3 ++- webapp/src/components/views/BackendProbe.tsx | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/webapp/src/checkEnv.ts b/webapp/src/checkEnv.ts index 2a2f2fbb3..f3b322c3a 100644 --- a/webapp/src/checkEnv.ts +++ b/webapp/src/checkEnv.ts @@ -1,5 +1,7 @@ -// Checks if all required environment variables are defined -// Returns an array of missing variables +/** + * Checks if all required environment variables are defined + * @returns {string[]} An array of missing environment variables + */ export const getMissingEnvVariables = () => { // Should be aligned with variables defined in .env.example const envVariables = ['REACT_APP_BACKEND_URI', 'REACT_APP_AAD_AUTHORITY', 'REACT_APP_AAD_CLIENT_ID']; diff --git a/webapp/src/components/token-usage/TokenUsageGraph.tsx b/webapp/src/components/token-usage/TokenUsageGraph.tsx index 697ab01be..4ba0529cb 100644 --- a/webapp/src/components/token-usage/TokenUsageGraph.tsx +++ b/webapp/src/components/token-usage/TokenUsageGraph.tsx @@ -63,7 +63,8 @@ const contrastColors = [ export const TokenUsageGraph: React.FC = ({ promptView, tokenUsage }) => { const classes = useClasses(); const { conversations, selectedId } = useAppSelector((state: RootState) => state.conversations); - const loadingResponse = conversations[selectedId].botResponseStatus && Object.entries(tokenUsage).length === 0; + const loadingResponse = + selectedId !== '' && conversations[selectedId].botResponseStatus && Object.entries(tokenUsage).length === 0; const responseGenerationView: TokenUsageView = {}; const memoryGenerationView: TokenUsageView = {}; diff --git a/webapp/src/components/views/BackendProbe.tsx b/webapp/src/components/views/BackendProbe.tsx index fdb5b30a2..266963b86 100644 --- a/webapp/src/components/views/BackendProbe.tsx +++ b/webapp/src/components/views/BackendProbe.tsx @@ -20,7 +20,7 @@ const BackendProbe: FC = ({ uri, onBackendFound }) => { } }; - void fetchAsync().catch(() => { + fetchAsync().catch(() => { // Ignore - this page is just a probe, so we don't need to show any errors if backend is not found }); }, 3000); From 88ba9fa91b2643ddd90713f2e49334058939b470 Mon Sep 17 00:00:00 2001 From: Teresa Hoang Date: Tue, 8 Aug 2023 17:38:51 -0700 Subject: [PATCH 3/3] Updating details about stepwise --- webapi/Options/PlannerOptions.cs | 2 +- webapi/appsettings.json | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/webapi/Options/PlannerOptions.cs b/webapi/Options/PlannerOptions.cs index 291533159..7d9c79d3f 100644 --- a/webapi/Options/PlannerOptions.cs +++ b/webapi/Options/PlannerOptions.cs @@ -33,7 +33,7 @@ public class MissingFunctionErrorOptions public const string PropertyName = "Planner"; /// - /// Define if the planner must be Sequential or not. + /// The type of planner to used to create plan. /// [Required] public PlanType Type { get; set; } = PlanType.Action; diff --git a/webapi/appsettings.json b/webapi/appsettings.json index d3a27c729..f7d7790f4 100644 --- a/webapi/appsettings.json +++ b/webapi/appsettings.json @@ -52,6 +52,7 @@ // - Set Planner:Type to "Action" to use the single-step ActionPlanner // - Set Planner:Type to "Sequential" to enable the multi-step SequentialPlanner // Note: SequentialPlanner works best with `gpt-4`. See the "Enabling Sequential Planner" section in webapi/README.md for configuration instructions. + // - Set Planner:Type to "Stepwise" to enable MRKL style planning // - Set Planner:RelevancyThreshold to a decimal between 0 and 1.0. // "Planner": {