Improve robustness when sending chat messages (#119)

app/components/chat/Messages.client.tsx

@@ -16,8 +16,18 @@ export const Messages = React.forwardRef<HTMLDivElement, MessagesProps>((props:
   const { id, hasPendingMessage = false, pendingMessageStatus = '', messages = [] } = props;
   const [showDetailMessageIds, setShowDetailMessageIds] = useState<string[]>([]);
 
+  // Get the last user response before a given message, or null if there is
+  // no user response between this and the last user message.
   const getLastUserResponse = (index: number) => {
-    return messages.findLast((message, messageIndex) => messageIndex < index && message.category === 'UserResponse');
+    for (let i = index - 1; i >= 0; i--) {
+      if (messages[i].category === 'UserResponse') {
+        return messages[i];
+      }
+      if (messages[i].role === 'user') {
+        return null;
+      }
+    }
+    return null;
   };
 
   // Return whether the test results at index are the last for the associated user response.
@@ -84,10 +94,7 @@ export const Messages = React.forwardRef<HTMLDivElement, MessagesProps>((props:
 
     if (!isUserMessage && message.category && message.category !== 'UserResponse') {
       const lastUserResponse = getLastUserResponse(index);
-      if (!lastUserResponse) {
-        return null;
-      }
-      const showDetails = showDetailMessageIds.includes(lastUserResponse.id);
+      const showDetails = !lastUserResponse || showDetailMessageIds.includes(lastUserResponse.id);
 
       if (message.category === TEST_RESULTS_CATEGORY) {
         // The default view only shows the last test results for each user response.
@@ -95,10 +102,8 @@ export const Messages = React.forwardRef<HTMLDivElement, MessagesProps>((props:
           return null;
         }
         return renderTestResults(message, index);
-      } else {
-        if (!showDetails) {
-          return null;
-        }
+      } else if (!showDetails) {
+        return null;
       }
     }
 

app/lib/replay/ChatManager.ts

@@ -11,6 +11,11 @@ import { database } from '~/lib/persistence/chats';
 import { chatStore } from '~/lib/stores/chat';
 import { debounce } from '~/utils/debounce';
 import { getSupabase } from '~/lib/supabase/client';
+import { pingTelemetry } from '~/lib/hooks/pingTelemetry';
+
+// We report to telemetry if we start a message and don't get any response
+// before this timeout.
+const ChatResponseTimeoutMs = 20_000;
 
 function createRepositoryIdPacket(repositoryId: string): SimulationPacket {
   return {
@@ -53,6 +58,9 @@ class ChatManager {
   // Simulation data for the page itself and any user interactions.
   pageData: SimulationData = [];
 
+  // Whether there were any errors in commands related to the simulation.
+  private hadSimulationError = false;
+
   constructor() {
     this.client = new ProtocolClient();
     this.chatIdPromise = (async () => {
@@ -89,6 +97,7 @@ class ChatManager {
       await this.client?.sendCommand({
         method: 'Nut.finishChat',
         params: { chatId },
+        errorHandled: true,
       });
     } catch (e) {
       console.error('Error finishing chat', e);
@@ -105,16 +114,21 @@ class ChatManager {
     const packet = createRepositoryIdPacket(repositoryId);
 
     const chatId = await this.chatIdPromise;
-    await this.client.sendCommand({
-      method: 'Nut.addSimulation',
-      params: {
-        chatId,
-        version: simulationDataVersion,
-        simulationData: [packet],
-        completeData: false,
-        saveRecording: true,
-      },
-    });
+    try {
+      await this.client.sendCommand({
+        method: 'Nut.addSimulation',
+        params: {
+          chatId,
+          version: simulationDataVersion,
+          simulationData: [packet],
+          completeData: false,
+          saveRecording: true,
+        },
+      });
+    } catch (e) {
+      console.log('Error adding simulation', e);
+      this.hadSimulationError = true;
+    }
   }
 
   async addPageData(data: SimulationData) {
@@ -135,10 +149,16 @@ class ChatManager {
 
     console.log('ChatAddPageData', new Date().toISOString(), chatId, data.length);
 
-    await this.client.sendCommand({
-      method: 'Nut.addSimulationData',
-      params: { chatId, simulationData: data },
-    });
+    try {
+      await this.client.sendCommand({
+        method: 'Nut.addSimulationData',
+        params: { chatId, simulationData: data },
+        errorHandled: true,
+      });
+    } catch (e) {
+      console.log('Error adding simulation data', e);
+      this.hadSimulationError = true;
+    }
   }
 
   async finishSimulationData() {
@@ -148,15 +168,57 @@ class ChatManager {
     this.simulationFinished = true;
 
     const chatId = await this.chatIdPromise;
-    await this.client.sendCommand({
-      method: 'Nut.finishSimulationData',
-      params: { chatId },
-    });
+    try {
+      await this.client.sendCommand({
+        method: 'Nut.finishSimulationData',
+        params: { chatId },
+        errorHandled: true,
+      });
+    } catch (e) {
+      console.log('Error finishing simulation data', e);
+      this.hadSimulationError = true;
+    }
+  }
+
+  // If we encounter a backend error during simulation the problem is likely that
+  // the underlying resources running the simulation were destroyed. Start a new
+  // chat and add all the simulation data again, which doesn't let us stream the
+  // data up but will work at least.
+  async regenerateChat() {
+    assert(this.client, 'Chat has been destroyed');
+
+    const { chatId } = (await this.client.sendCommand({ method: 'Nut.startChat', params: {} })) as { chatId: string };
+    console.log('RegenerateSimulationChat', new Date().toISOString(), chatId);
+
+    this.chatIdPromise = Promise.resolve(chatId);
+
+    if (this.repositoryId) {
+      const packet = createRepositoryIdPacket(this.repositoryId);
+
+      await this.client.sendCommand({
+        method: 'Nut.addSimulation',
+        params: {
+          chatId,
+          simulationData: [packet, ...this.pageData],
+          completeData: true,
+          saveRecording: true,
+        },
+      });
+    }
   }
 
   async sendChatMessage(messages: Message[], references: ChatReference[], callbacks: ChatMessageCallbacks) {
     assert(this.client, 'Chat has been destroyed');
 
+    const timeout = setTimeout(() => {
+      pingTelemetry('ChatMessageTimeout', { hasRepository: !!this.repositoryId });
+    }, ChatResponseTimeoutMs);
+
+    if (this.hadSimulationError) {
+      this.hadSimulationError = false;
+      await this.regenerateChat();
+    }
+
     const responseId = `response-${generateRandomId()}`;
 
     const removeResponseListener = this.client.listenForMessage(
@@ -164,6 +226,7 @@ class ChatManager {
       ({ responseId: eventResponseId, message }: { responseId: string; message: Message }) => {
         if (responseId == eventResponseId) {
           console.log('ChatResponse', chatId, message);
+          clearTimeout(timeout);
           callbacks.onResponsePart(message);
         }
       },

app/lib/replay/ReplayProtocolClient.ts

@@ -94,7 +94,7 @@ export class ProtocolClient {
   openDeferred = createDeferred<void>();
   eventListeners = new Map<string, Set<EventListener>>();
   nextMessageId = 1;
-  pendingCommands = new Map<number, { method: string; deferred: Deferred<any> }>();
+  pendingCommands = new Map<number, { method: string; deferred: Deferred<any>; errorHandled: boolean }>();
   socket: WebSocket;
 
   constructor() {
@@ -141,10 +141,10 @@ export class ProtocolClient {
     };
   }
 
-  sendCommand(args: { method: string; params: any; sessionId?: string }) {
+  sendCommand(args: { method: string; params: any; sessionId?: string; errorHandled?: boolean }) {
     const id = this.nextMessageId++;
 
-    const { method, params, sessionId } = args;
+    const { method, params, sessionId, errorHandled = false } = args;
     logDebug('Sending command', { id, method, params, sessionId });
 
     const command = {
@@ -157,7 +157,7 @@ export class ProtocolClient {
     this.socket.send(JSON.stringify(command));
 
     const deferred = createDeferred();
-    this.pendingCommands.set(id, { method, deferred });
+    this.pendingCommands.set(id, { method, deferred, errorHandled });
 
     return deferred.promise;
   }
@@ -182,8 +182,12 @@ export class ProtocolClient {
       if (result) {
         info.deferred.resolve(result);
       } else if (error) {
-        pingTelemetry('ProtocolError', { method: info.method, error });
-        console.error('ProtocolError', info.method, id, error);
+        if (info.errorHandled) {
+          console.log('ProtocolErrorHandled', info.method, id, error);
+        } else {
+          pingTelemetry('ProtocolError', { method: info.method, error });
+          console.error('ProtocolError', info.method, id, error);
+        }
         info.deferred.reject(new ProtocolError(error));
       } else {
         info.deferred.reject(new Error('Channel error'));

eslint.config.mjs

@@ -13,6 +13,7 @@ export default [
       '@typescript-eslint/no-this-alias': 'off',
       '@typescript-eslint/no-empty-object-type': 'off',
       '@typescript-eslint/no-empty-function': 'off',
+      '@typescript-eslint/naming-convention': 'off',
       '@blitz/comment-syntax': 'off',
       '@blitz/block-scope-case': 'off',
       'array-bracket-spacing': ['error', 'never'],