Fix support feedback error handling and improve API validation

GiselleNessi · GiselleNessi · commit 33d32468b954 · 2025-09-09T16:57:56.000+02:00
diff --git a/apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/support/_components/SupportCaseDetails.tsx b/apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/support/_components/SupportCaseDetails.tsx
@@ -47,6 +47,46 @@ function SupportCaseDetails({ ticket, team }: SupportCaseDetailsProps) {
   // rating/feedback
   const [rating, setRating] = useState(0);
   const [feedback, setFeedback] = useState("");
+  // non-blocking warning when status check fails
+  const [statusCheckFailed, setStatusCheckFailed] = useState(false);
+
+  // Helper function to handle status check errors consistently
+  const handleStatusCheckError = (
+    error: unknown,
+    logContext: Record<string, string | number | boolean>,
+  ) => {
+    const errorMessage = error instanceof Error ? error.message : String(error);
+    const errorType = /network|fetch/i.test(errorMessage)
+      ? "network"
+      : /API Server error/i.test(errorMessage)
+        ? "server"
+        : /Internal server error/i.test(errorMessage)
+          ? "server"
+          : "unknown";
+
+    // Report analytics for status check failure
+    reportSupportFeedbackStatusCheckFailed({
+      ticketId: ticket.id,
+      errorMessage,
+      errorType,
+    });
+
+    // Set degraded state for warning banner
+    setStatusCheckFailed(true);
+
+    // Log the error
+    console.warn(
+      "[SUPPORT_FEEDBACK_CLIENT] Status check failed, but allowing feedback submission",
+      {
+        ...logContext,
+        error: errorMessage,
+      },
+    );
+
+    // Throw a custom error that React Query will catch, but the UI will handle gracefully
+    // This prevents treating the error as "no feedback exists" which could cause duplicates
+    throw new Error(`Status check failed: ${errorMessage}`);
+  };
 
   // Check if feedback has already been submitted for this ticket
   const feedbackStatusQuery = useQuery({
@@ -75,32 +115,8 @@ function SupportCaseDetails({ ticket, team }: SupportCaseDetailsProps) {
             },
           );
 
-          // Report analytics for status check failure
-          const errorType = /network|fetch/i.test(result.error)
-            ? "network"
-            : /API Server error/i.test(result.error)
-              ? "server"
-              : /Internal server error/i.test(result.error)
-                ? "server"
-                : "unknown";
-
-          reportSupportFeedbackStatusCheckFailed({
-            ticketId: ticket.id,
-            errorMessage: result.error,
-            errorType,
-          });
-
-          // Don't throw error for status check failures - allow user to still submit feedback
-          console.warn(
-            "[SUPPORT_FEEDBACK_CLIENT] Status check failed, but allowing feedback submission",
-            {
-              ...logContext,
-              error: result.error,
-            },
-          );
-
-          // Return false to indicate no feedback found (allow submission)
-          return false;
+          // Use shared error handler to maintain consistent behavior
+          return handleStatusCheckError(result.error, logContext);
         }
 
         console.log(
@@ -112,6 +128,9 @@ function SupportCaseDetails({ ticket, team }: SupportCaseDetailsProps) {
           },
         );
 
+        // Clear degraded state on success
+        if (statusCheckFailed) setStatusCheckFailed(false);
+
         // Additional debugging for production
         if (result.hasFeedback) {
           console.log(
@@ -139,34 +158,8 @@ function SupportCaseDetails({ ticket, team }: SupportCaseDetailsProps) {
           errorName: error instanceof Error ? error.name : "Unknown",
         });
 
-        // Report analytics for status check failure
-        const errorType = /network|fetch/i.test(
-          error instanceof Error ? error.message : String(error),
-        )
-          ? "network"
-          : /API Server error/i.test(
-                error instanceof Error ? error.message : String(error),
-              )
-            ? "server"
-            : "unknown";
-
-        reportSupportFeedbackStatusCheckFailed({
-          ticketId: ticket.id,
-          errorMessage: error instanceof Error ? error.message : String(error),
-          errorType,
-        });
-
-        // Don't throw error - allow user to still submit feedback
-        console.warn(
-          "[SUPPORT_FEEDBACK_CLIENT] Status check failed with exception, but allowing feedback submission",
-          {
-            ...logContext,
-            error: error instanceof Error ? error.message : String(error),
-          },
-        );
-
-        // Return false to indicate no feedback found (allow submission)
-        return false;
+        // Use shared error handler to maintain consistent behavior
+        return handleStatusCheckError(error, logContext);
       }
     },
     enabled: ticket.status === "closed",
@@ -176,7 +169,8 @@ function SupportCaseDetails({ ticket, team }: SupportCaseDetailsProps) {
 
   const feedbackSubmitted = feedbackStatusQuery.data ?? false;
   const isLoading = feedbackStatusQuery.isLoading;
-  const hasError = feedbackStatusQuery.isError;
+  // query never throws; use local degraded flag for the inline warning
+  const hasError = statusCheckFailed;
 
   const handleStarClick = (starIndex: number) => {
     setRating(starIndex + 1);
@@ -508,75 +502,77 @@ function SupportCaseDetails({ ticket, team }: SupportCaseDetailsProps) {
           </div>
         )}
 
-        {ticket.status === "closed" && !isLoading && !feedbackSubmitted && (
-          <div className="border-t p-6">
-            <p className="text-muted-foreground text-sm">
-              This ticket is closed. Give us a quick rating to let us know how
-              we did!
-            </p>
-            {hasError && (
-              <div className="mt-2">
-                <p className="text-destructive text-xs">
-                  Couldn't verify prior feedback right now — you can still
-                  submit a rating.
-                </p>
+        {ticket.status === "closed" &&
+          !isLoading &&
+          (!feedbackSubmitted || hasError) && (
+            <div className="border-t p-6">
+              <p className="text-muted-foreground text-sm">
+                This ticket is closed. Give us a quick rating to let us know how
+                we did!
+              </p>
+              {hasError && (
+                <div className="mt-2">
+                  <p className="text-destructive text-xs">
+                    Couldn't verify prior feedback right now — you can still
+                    submit a rating.
+                  </p>
+                </div>
+              )}
+
+              <div className="flex gap-2 mb-6 mt-4">
+                {[1, 2, 3, 4, 5].map((starValue) => (
+                  <button
+                    key={`star-${starValue}`}
+                    type="button"
+                    onClick={() => handleStarClick(starValue - 1)}
+                    className="transition-colors"
+                    aria-label={`Rate ${starValue} out of 5 stars`}
+                  >
+                    <StarIcon
+                      size={32}
+                      className={cn(
+                        "transition-colors",
+                        starValue <= rating
+                          ? "text-pink-500 fill-current stroke-current"
+                          : "text-muted-foreground fill-current stroke-current",
+                        "hover:text-pink-500",
+                      )}
+                      strokeWidth={starValue <= rating ? 2 : 1}
+                    />
+                  </button>
+                ))}
               </div>
-            )}
 
-            <div className="flex gap-2 mb-6 mt-4">
-              {[1, 2, 3, 4, 5].map((starValue) => (
-                <button
-                  key={`star-${starValue}`}
+              <div className="relative">
+                <AutoResizeTextarea
+                  value={feedback}
+                  onChange={(e) => setFeedback(e.target.value)}
+                  placeholder="Optional: Tell us how we can improve."
+                  maxLength={1000}
+                  className="text-sm w-full bg-card text-foreground rounded-lg p-4 pr-28 min-h-[100px] resize-none border border-border focus:outline-none placeholder:text-muted-foreground"
+                />
+                <Button
                   type="button"
-                  onClick={() => handleStarClick(starValue - 1)}
-                  className="transition-colors"
-                  aria-label={`Rate ${starValue} out of 5 stars`}
+                  onClick={handleSendFeedback}
+                  disabled={submitFeedbackMutation.isPending || rating === 0}
+                  className="absolute bottom-3 right-3 rounded-full h-auto py-2 px-4"
+                  variant="secondary"
+                  size="sm"
                 >
-                  <StarIcon
-                    size={32}
-                    className={cn(
-                      "transition-colors",
-                      starValue <= rating
-                        ? "text-pink-500 fill-current stroke-current"
-                        : "text-muted-foreground fill-current stroke-current",
-                      "hover:text-pink-500",
-                    )}
-                    strokeWidth={starValue <= rating ? 2 : 1}
-                  />
-                </button>
-              ))}
-            </div>
-
-            <div className="relative">
-              <AutoResizeTextarea
-                value={feedback}
-                onChange={(e) => setFeedback(e.target.value)}
-                placeholder="Optional: Tell us how we can improve."
-                maxLength={1000}
-                className="text-sm w-full bg-card text-foreground rounded-lg p-4 pr-28 min-h-[100px] resize-none border border-border focus:outline-none placeholder:text-muted-foreground"
-              />
-              <Button
-                type="button"
-                onClick={handleSendFeedback}
-                disabled={submitFeedbackMutation.isPending || rating === 0}
-                className="absolute bottom-3 right-3 rounded-full h-auto py-2 px-4"
-                variant="secondary"
-                size="sm"
-              >
-                {submitFeedbackMutation.isPending ? (
-                  <>
-                    <Spinner className="size-4 mr-2" />
-                    Sending...
-                  </>
-                ) : (
-                  "Send Feedback"
-                )}
-              </Button>
+                  {submitFeedbackMutation.isPending ? (
+                    <>
+                      <Spinner className="size-4 mr-2" />
+                      Sending...
+                    </>
+                  ) : (
+                    "Send Feedback"
+                  )}
+                </Button>
+              </div>
             </div>
-          </div>
-        )}
+          )}
 
-        {ticket.status === "closed" && feedbackSubmitted && (
+        {ticket.status === "closed" && feedbackSubmitted && !hasError && (
           <div className="border-t p-6">
             <p className="text-muted-foreground text-sm">
               Thank you for your feedback! We appreciate your input and will use
diff --git a/apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/support/apis/feedback.ts b/apps/dashboard/src/app/(app)/team/[team_slug]/(team)/~/support/apis/feedback.ts
@@ -13,7 +13,16 @@ export async function submitSupportFeedback({
   ticketId: string;
 }): Promise<FeedbackSubmitResult> {
   try {
-    // Fail fast on missing configuration
+    // Basic input validation first - fail fast on invalid input
+    if (!ticketId?.trim()) {
+      return { error: "ticketId is required." };
+    }
+
+    if (!Number.isInteger(rating) || rating < 1 || rating > 5) {
+      return { error: "Rating must be an integer between 1 and 5." };
+    }
+
+    // Configuration validation after input validation
     const siwaUrl =
       process.env.SIWA_URL ?? process.env.NEXT_PUBLIC_SIWA_URL ?? "";
 
@@ -27,15 +36,6 @@ export async function submitSupportFeedback({
       throw new Error("SERVICE_AUTH_KEY_SIWA not configured");
     }
 
-    // Basic input validation/normalization
-    if (!ticketId?.trim()) {
-      return { error: "ticketId is required." };
-    }
-
-    if (!Number.isInteger(rating) || rating < 1 || rating > 5) {
-      return { error: "Rating must be an integer between 1 and 5." };
-    }
-
     const normalizedFeedback = (feedback ?? "")
       .toString()
       .trim()
@@ -76,12 +76,8 @@ export async function submitSupportFeedback({
     return { success: true };
   } catch (error) {
     if (error instanceof Error) {
-      if (error.name === "AbortError") {
-        return { error: "Request timeout" };
-      }
-      if (error.message.includes("fetch")) {
-        return { error: "Network error" };
-      }
+      if (error.name === "AbortError") return { error: "Request timeout" };
+      if (error instanceof TypeError) return { error: "Network error" };
       return { error: error.message };
     }
     return { error: "Unknown error occurred" };
@@ -92,6 +88,12 @@ export async function checkFeedbackStatus(
   ticketId: string,
 ): Promise<FeedbackStatusResult> {
   try {
+    // Basic input validation first - fail fast on invalid input
+    if (!ticketId?.trim()) {
+      return { error: "ticketId is required." };
+    }
+
+    // Configuration validation after input validation
     const siwaUrl =
       process.env.SIWA_URL ?? process.env.NEXT_PUBLIC_SIWA_URL ?? "";
 
@@ -105,10 +107,6 @@ export async function checkFeedbackStatus(
       throw new Error("SERVICE_AUTH_KEY_SIWA not configured");
     }
 
-    if (!ticketId?.trim()) {
-      return { error: "ticketId is required." };
-    }
-
     const ac = new AbortController();
     const t = setTimeout(() => ac.abort(), 10_000);
 
@@ -150,19 +148,28 @@ export async function checkFeedbackStatus(
       return { error: "Invalid JSON response from API" };
     }
 
-    if (typeof data.has_feedback !== "boolean") {
+    // Comprehensive validation of the API response structure
+    if (
+      typeof data.has_feedback !== "boolean" ||
+      (data.feedback_data !== null &&
+        (typeof data.feedback_data !== "object" ||
+          typeof data.feedback_data.id !== "string" ||
+          (data.feedback_data.rating !== null &&
+            typeof data.feedback_data.rating !== "number") ||
+          (data.feedback_data.feedback !== null &&
+            typeof data.feedback_data.feedback !== "string") ||
+          (data.feedback_data.ticket_id !== null &&
+            typeof data.feedback_data.ticket_id !== "string") ||
+          typeof data.feedback_data.created_at !== "string"))
+    ) {
       return { error: "Invalid response format from API" };
     }
 
     return { hasFeedback: data.has_feedback };
   } catch (error) {
     if (error instanceof Error) {
-      if (error.name === "AbortError") {
-        return { error: "Request timeout" };
-      }
-      if (error.message.includes("fetch")) {
-        return { error: "Network error" };
-      }
+      if (error.name === "AbortError") return { error: "Request timeout" };
+      if (error instanceof TypeError) return { error: "Network error" };
       return { error: error.message };
     }
     return { error: "Unknown error occurred" };
