Skip to content

ECHO-428 Portal "Conversation Ended" fixed #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ussaama merged 5 commits into from
Aug 28, 2025
Merged

ECHO-428 Portal "Conversation Ended" fixed #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ebd7509
- added specific error codes to make the conversation ended true
ussaama Aug 28, 2025
14f7f0c
- fixes
ussaama Aug 28, 2025
ecb449d
- fixes
ussaama Aug 28, 2025
aaab4c8
- increase retry count to 6 times
ussaama Aug 28, 2025
b811722
- upload chunks retry increased to 20 times
ussaama Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions echo/frontend/src/components/participant/hooks/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { createItem, readItems } from "@directus/sdk";
import { useMutation, useQueryClient } from "@tanstack/react-query";
import { toast } from "@/components/common/Toaster";
import { useQuery } from "@tanstack/react-query";
import { AxiosError } from "axios";
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Import is fine; inline type on retry can be avoided.

You can drop the inline AxiosError param typing by typing the query generics instead (see suggestion below). Keeps the import, but avoids repeating the type in function params.

🤖 Prompt for AI Agents 
In echo/frontend/src/components/participant/hooks/index.ts around line 16, the
review notes that the inline AxiosError parameter typing can be removed by
supplying the appropriate generics to the query/mutation call instead; keep the
existing AxiosError import, remove the explicit AxiosError type annotation from
the callback parameter, and set the query/mutation generic type parameters
(e.g., <Data, AxiosError, Variables>) so the error type is inferred without
repeating it in the function signature.


export const useCreateProjectReportMetricOncePerDayMutation = () => {
return useMutation({
Expand Down Expand Up @@ -55,7 +56,7 @@ export const useUploadConversationChunk = () => {

return useMutation({
mutationFn: uploadConversationChunk,
retry: 10,
retry: 20,
// When mutate is called:
Comment on lines +59 to 60
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Blindly bumping retries to 20 will DDoS your own API under load; make retries status-aware with exponential backoff + jitter.

Aligns with ECHO-428 (contention during concurrent recordings). Gate on non-transient 4xx and back off.

   return useMutation({
     mutationFn: uploadConversationChunk,
-    retry: 20,
+    retry: (failureCount, error) => {
+      const status = isAxiosError(error) ? error.response?.status : undefined;
+      // Don't retry on non-transient client errors
+      if (typeof status === "number" && [400, 401, 403, 404, 409, 410, 422].includes(status)) {
+        return false;
+      }
+      return failureCount < 8;
+    },
+    retryDelay: (attempt) =>
+      Math.min(30000, 1000 * 2 ** attempt) + Math.floor(Math.random() * 500),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
retry: 20,
// When mutate is called:
return useMutation({
mutationFn: uploadConversationChunk,
retry: (failureCount, error) => {
const status = isAxiosError(error) ? error.response?.status : undefined;
// Don't retry on non-transient client errors
if (
typeof status === "number" &&
[400, 401, 403, 404, 409, 410, 422].includes(status)
) {
return false;
}
// Retry up to 8 times for transient failures
return failureCount < 8;
},
retryDelay: (attempt) =>
Math.min(30000, 1000 * 2 ** attempt) + Math.floor(Math.random() * 500),
// When mutate is called:
…
});
🤖 Prompt for AI Agents 
In echo/frontend/src/components/participant/hooks/index.ts around lines 59-60,
the retry count is hard-coded to 20 which can DDoS our API; change retry logic
to be status-aware and use exponential backoff with jitter: detect non-transient
4xx responses and do not retry them, retry transient errors (5xx, network
timeouts) with an increasing delay (baseDelay * 2^attempt) plus random jitter,
and cap the max attempts and max delay; implement this in the mutate/retry
handler so the backoff calculation and status checks determine whether to retry
rather than a fixed retry count.

onMutate: async (variables) => {
// Cancel any outgoing refetches
Expand Down Expand Up @@ -278,7 +279,6 @@ export const useSubmitNotificationParticipant = () => {
});
};


export const useParticipantProjectById = (projectId: string) => {
return useQuery({
queryKey: ["participantProject", projectId],
Expand Down Expand Up @@ -308,7 +308,9 @@ export const combineUserChunks = (
};
};

export const useConversationRepliesQuery = (conversationId: string | undefined) => {
export const useConversationRepliesQuery = (
conversationId: string | undefined,
) => {
return useQuery({
queryKey: ["participant", "conversation_replies", conversationId],
queryFn: () =>
Expand All @@ -330,9 +332,19 @@ export const useConversationQuery = (
) => {
return useQuery({
queryKey: ["participant", "conversation", projectId, conversationId],
queryFn: () => getParticipantConversationById(projectId ?? "", conversationId ?? ""),
queryFn: () =>
getParticipantConversationById(projectId ?? "", conversationId ?? ""),
enabled: !!conversationId && !!projectId,
refetchInterval: 60000,
retry: (failureCount, error: AxiosError) => {
const status = error?.response?.status;
// Don't retry if conversation is deleted
if (status && [404, 403, 410].includes(status as number)) {
return false;
}
Comment on lines +341 to +344
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Deduplicate deletion status codes (now used in ≥2 places).

These codes also live in ParticipantConversation.tsx; centralize to prevent drift.

+import { CONVERSATION_DELETION_STATUS_CODES } from "@/constants/http";
@@
-      if (typeof status === "number" && [404, 403, 410].includes(status)) {
+      if (typeof status === "number" && CONVERSATION_DELETION_STATUS_CODES.includes(status)) {
         return false;
       }

Add a tiny constant (zero “architecture” overhead):

// echo/frontend/src/constants/http.ts
export const CONVERSATION_DELETION_STATUS_CODES = [404, 403, 410] as const;

🏁 Script executed:

#!/bin/bash
# Evidence of duplication to justify the constant
rg -nC2 'CONVERSATION_DELETION_STATUS_CODES|\\[(404|403|410)\\]' --type=ts

Length of output: 237318

Centralize conversation deletion HTTP status codes
Extract [404, 403, 410] into echo/frontend/src/constants/http.ts as

export const CONVERSATION_DELETION_STATUS_CODES = [404, 403, 410] as const;

Then in both files…

  • src/components/participant/hooks/index.ts (around lines 341–344)
  • src/routes/participant/ParticipantConversation.tsx (remove its local CONVERSATION_DELETION_STATUS_CODES declaration)
    …replace literal checks with
import { CONVERSATION_DELETION_STATUS_CODES } from "@/constants/http";
…
if (typeof status === "number" && CONVERSATION_DELETION_STATUS_CODES.includes(status)) {
  return false;
}
🤖 Prompt for AI Agents 
In echo/frontend/src/components/participant/hooks/index.ts around lines 341–344,
extract the literal array [404, 403, 410] into
echo/frontend/src/constants/http.ts as a named export
CONVERSATION_DELETION_STATUS_CODES, then import that constant here and replace
the current check with a type-safe number check that uses
CONVERSATION_DELETION_STATUS_CODES.includes(status); also remove the local
CONVERSATION_DELETION_STATUS_CODES declaration from
src/routes/participant/ParticipantConversation.tsx and update that file to
import and use the shared constant instead so both places use the centralized
export.


return failureCount < 6;
},
Comment on lines +335 to +347
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Narrow status without casts; kill the as number.

Safer type guard; no functional change.

-    retry: (failureCount, error: AxiosError) => {
-      const status = error?.response?.status;
-      // Don't retry if conversation is deleted
-      if (status && [404, 403, 410].includes(status as number)) {
+    retry: (failureCount, error: AxiosError) => {
+      const status = error?.response?.status;
+      // Don't retry if conversation is deleted/forbidden/gone
+      if (typeof status === "number" && [404, 403, 410].includes(status)) {
         return false;
       }
-
       return failureCount < 6;
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
queryFn: () =>
getParticipantConversationById(projectId ?? "", conversationId ?? ""),
enabled: !!conversationId && !!projectId,
refetchInterval: 60000,
retry: (failureCount, error: AxiosError) => {
const status = error?.response?.status;
// Don't retry if conversation is deleted
if (status && [404, 403, 410].includes(status as number)) {
return false;
}
return failureCount < 6;
},
queryFn: () =>
getParticipantConversationById(projectId ?? "", conversationId ?? ""),
enabled: !!conversationId && !!projectId,
refetchInterval: 60000,
retry: (failureCount, error: AxiosError) => {
const status = error?.response?.status;
// Don't retry if conversation is deleted/forbidden/gone
if (typeof status === "number" && [404, 403, 410].includes(status)) {
return false;
}
return failureCount < 6;
},
🤖 Prompt for AI Agents 
In echo/frontend/src/components/participant/hooks/index.ts around lines 335 to
347, the retry handler narrows the AxiosError response status using an
unnecessary type cast "as number"; replace that cast with a proper type guard so
you only check numeric statuses (e.g. ensure status is a number before checking
inclusion in [404, 403, 410]). Update the conditional to first verify typeof
status === "number" (or use Number.isInteger/status != null && typeof status ===
"number") and then test the array membership, removing the "as number" cast.

Comment on lines 338 to +347
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Stop polling after 404/403/410 to avoid minute-by-minute error pings.

Disable refetchInterval when the last error is a deletion code.

-    refetchInterval: 60000,
+    refetchInterval: (_data, query) => {
+      const err = query.state.error;
+      const status =
+        (typeof (err as any)?.response?.status === "number"
+          ? (err as any).response.status
+          : undefined);
+      return typeof status === "number" && [404, 403, 410].includes(status) ? false : 60000;
+    },

If you prefer a type-safe guard, import isAxiosError:

- import { AxiosError } from "axios";
+ import { AxiosError, isAxiosError } from "axios";

And then:

-      const status =
-        (typeof (err as any)?.response?.status === "number"
-          ? (err as any).response.status
-          : undefined);
+      const status = isAxiosError(err) ? err.response?.status : undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
refetchInterval: 60000,
retry: (failureCount, error: AxiosError) => {
const status = error?.response?.status;
// Don't retry if conversation is deleted
if (status && [404, 403, 410].includes(status as number)) {
return false;
}
return failureCount < 6;
},
import { AxiosError, isAxiosError } from "axios";
refetchInterval: (_data, query) => {
const err = query.state.error;
const status = isAxiosError(err) ? err.response?.status : undefined;
// Stop polling on deletion/forbidden/gone
return typeof status === "number" && [404, 403, 410].includes(status)
? false
: 60000;
},

});
};

Expand All @@ -346,4 +358,4 @@ export const useConversationChunksQuery = (
getParticipantConversationChunks(projectId ?? "", conversationId ?? ""),
refetchInterval: 60000,
});
};
};
37 changes: 30 additions & 7 deletions echo/frontend/src/routes/participant/ParticipantConversation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import useChunkedAudioRecorder from "@/components/participant/hooks/useChunkedAu
import { EchoErrorAlert } from "@/components/participant/EchoErrorAlert";

const DEFAULT_REPLY_COOLDOWN = 120; // 2 minutes in seconds
const CONVERSATION_DELETION_STATUS_CODES = [404, 403, 410];

export const ParticipantConversationAudioRoute = () => {
const { projectId, conversationId } = useParams();
Expand Down Expand Up @@ -164,17 +165,39 @@ export const ParticipantConversationAudioRoute = () => {

// Monitor conversation status during recording - handle deletion mid-recording
useEffect(() => {
if (isRecording && (conversationQuery.isError || !conversationQuery.data)) {
console.warn(
"Conversation deleted or became unavailable during recording",
);
stopRecording();
setConversationDeletedDuringRecording(true);
if (!isRecording) return;

if (
conversationQuery.isError &&
!conversationQuery.isFetching &&
!conversationQuery.isLoading
) {
const error = conversationQuery.error;
const httpStatus = error?.response?.status;

if (
httpStatus &&
CONVERSATION_DELETION_STATUS_CODES.includes(httpStatus)
) {
console.warn(
"Conversation was deleted or is no longer accessible during recording",
{ status: httpStatus, message: error?.message },
);
stopRecording();
setConversationDeletedDuringRecording(true);
} else {
console.warn(
"Error fetching conversation during recording - continuing",
{ status: httpStatus, message: error?.message },
);
}
}
}, [
isRecording,
conversationQuery.isError,
conversationQuery.data,
conversationQuery.isLoading,
conversationQuery.isFetching,
conversationQuery.error,
stopRecording,
]);

Expand Down