Improve error handling and cleanup logic

- Remove redundant abortController cleanup code - Add finally block to ensure proper state reset after requests - Add validation for trace_logs field in request data
getsentry · Nov 5, 2024 · 5bee9bc · 5bee9bc
1 parent bb15e45
commit 5bee9bc
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 29 deletions.
diff --git a/snuba/admin/static/rpc_endpoints/index.tsx b/snuba/admin/static/rpc_endpoints/index.tsx
@@ -39,7 +39,6 @@ function RpcEndpoints() {
   const handleEndpointSelect = (value: string | null) => {
     if (abortControllerRef.current) {
       abortControllerRef.current.abort();
-      abortControllerRef.current = null;
     }
 
     setSelectedEndpoint(value);
@@ -98,17 +97,23 @@ function RpcEndpoints() {
       alert(`Error: ${error.message}`);
       setResponse({ error: error.message });
       setIsLoading(false);
+    } finally {
+      if (abortControllerRef.current === controller) {
+        abortControllerRef.current = null;
+      }
+      setIsLoading(false);
     }
   };
 
   useEffect(() => {
     return () => {
       if (abortControllerRef.current) {
         abortControllerRef.current.abort();
-        setResponse(null);
-        setSummarizedTraceOutput(null);
-        setProfileEvents(null);
+        abortControllerRef.current = null;
       }
+      setResponse(null);
+      setSummarizedTraceOutput(null);
+      setProfileEvents(null);
     };
   }, []);
 

diff --git a/snuba/admin/views.py b/snuba/admin/views.py
@@ -614,34 +614,58 @@ def summarize_trace_with_profile() -> Response:
     try:
         req = json.loads(request.data)
         trace_logs = req.get("trace_logs")
+        storage = req.get("storage", "default")
 
-        if trace_logs:
-            summarized_trace_output = summarize_trace_output(trace_logs)
-            trace_output = TraceOutput(
-                trace_output=trace_logs,
-                summarized_trace_output=summarized_trace_output,
-                cols=[],
-                num_rows_result=0,
-                result=[],
-                profile_events_results={},
-                profile_events_meta=[],
-                profile_events_profile={},
+        if trace_logs is None:
+            return make_response(
+                jsonify(
+                    {
+                        "error": {
+                            "type": "validation",
+                            "message": "Missing required field: trace_logs",
+                        }
+                    }
+                ),
+                400,
             )
-            storage = req.get("storage", "default")
-            gather_profile_events(trace_output, storage)
-            return make_response(jsonify(asdict(trace_output)), 200)
-        else:
+        if not isinstance(trace_logs, str):
             return make_response(
                 jsonify(
                     {
                         "error": {
-                            "type": "request",
-                            "message": "Invalid request. Must provide either (trace_logs) or (storage, sql)",
+                            "type": "validation",
+                            "message": "trace_logs must be a string",
                         }
                     }
                 ),
                 400,
             )
+        if not trace_logs.strip():
+            return make_response(
+                jsonify(
+                    {
+                        "error": {
+                            "type": "validation",
+                            "message": "trace_logs cannot be empty",
+                        }
+                    }
+                ),
+                400,
+            )
+
+        summarized_trace_output = summarize_trace_output(trace_logs)
+        trace_output = TraceOutput(
+            trace_output=trace_logs,
+            summarized_trace_output=summarized_trace_output,
+            cols=[],
+            num_rows_result=0,
+            result=[],
+            profile_events_results={},
+            profile_events_meta=[],
+            profile_events_profile={},
+        )
+        gather_profile_events(trace_output, storage)
+        return make_response(jsonify(asdict(trace_output)), 200)
     except InvalidCustomQuery as err:
         return make_response(
             jsonify(
@@ -656,17 +680,16 @@ def summarize_trace_with_profile() -> Response:
         )
     except ClickhouseError as err:
         logger.error(err, exc_info=True)
-        details = {
-            "type": "clickhouse",
-            "message": str(err),
-            "code": err.code,
-        }
-        return make_response(jsonify({"error": details}), 400)
+        return make_response(
+            jsonify(
+                {"error": {"type": "clickhouse", "message": str(err), "code": err.code}}
+            ),
+            400,
+        )
     except Exception as err:
         logger.error(err, exc_info=True)
         return make_response(
-            jsonify({"error": {"type": "unknown", "message": str(err)}}),
-            500,
+            jsonify({"error": {"type": "unknown", "message": str(err)}}), 500
         )