Browse Source

refactor(ErrorHandling): streamline error handling in API responses and improve SSE performance data transmission

Jacky 3 days ago
parent
commit
c36128d31e

+ 3 - 7
api/nginx/status.go

@@ -38,7 +38,7 @@ func GetDetailStatus(c *gin.Context) {
 func StreamDetailStatus(c *gin.Context) {
 	// Set SSE response headers
 	api.SetSSEHeaders(c)
-	
+
 	// Create context that cancels when client disconnects
 	ctx := c.Request.Context()
 
@@ -54,10 +54,7 @@ func StreamDetailStatus(c *gin.Context) {
 		select {
 		case <-ticker.C:
 			// Send performance data
-			if err := sendPerformanceData(c); err != nil {
-				logger.Warn("Error sending SSE data:", err)
-				return
-			}
+			sendPerformanceData(c)
 		case <-ctx.Done():
 			// Client closed connection or request canceled
 			logger.Debug("Client closed connection")
@@ -67,7 +64,7 @@ func StreamDetailStatus(c *gin.Context) {
 }
 
 // sendPerformanceData sends performance data once
-func sendPerformanceData(c *gin.Context) error {
+func sendPerformanceData(c *gin.Context) {
 	response := performance.GetPerformanceData()
 
 	// Send SSE event
@@ -75,7 +72,6 @@ func sendPerformanceData(c *gin.Context) error {
 
 	// Flush buffer to ensure data is sent immediately
 	c.Writer.Flush()
-	return nil
 }
 
 // CheckStubStatus gets Nginx stub_status module status

+ 28 - 33
app/src/lib/http/error.ts

@@ -26,36 +26,8 @@ export function useMessageDedupe(interval = 5000): MessageDedupe {
   }
 }
 
-export function handleApiError(err: CosyError, dedupe: MessageDedupe) {
-  if (err?.scope) {
-    // check if already register
-    if (!errors[err.scope]) {
-      try {
-        // Dynamic import error files
-        import(`@/constants/errors/${err.scope}.ts`)
-          .then(error => {
-            registerError(err.scope!, error.default)
-            displayErrorMessage(err, dedupe)
-          })
-          .catch(err => {
-            console.error(err)
-            dedupe.error($gettext(err?.message ?? 'Server error'))
-          })
-      }
-      catch {
-        dedupe.error($gettext(err?.message ?? 'Server error'))
-      }
-    }
-    else {
-      displayErrorMessage(err, dedupe)
-    }
-  }
-  else {
-    dedupe.error($gettext(err?.message ?? 'Server error'))
-  }
-}
-
-function displayErrorMessage(err: CosyError, dedupe: MessageDedupe) {
+// Synchronous version for already registered errors
+function translateErrorSync(err: CosyError): string {
   const msg = errors?.[err.scope ?? '']?.[err.code ?? '']
 
   if (msg) {
@@ -67,13 +39,36 @@ function displayErrorMessage(err: CosyError, dedupe: MessageDedupe) {
         res = res.replaceAll(`{${index}}`, param)
       })
 
-      dedupe.error(res)
+      return res
     }
     else {
-      dedupe.error(msg())
+      return msg()
     }
   }
   else {
-    dedupe.error($gettext(err?.message ?? 'Server error'))
+    return $gettext(err?.message ?? 'Server error')
   }
 }
+
+// Asynchronous version that handles dynamic loading
+export async function translateError(err: CosyError): Promise<string> {
+  // If scope exists, use sync version
+  if (!err?.scope || errors[err.scope]) {
+    return translateErrorSync(err)
+  }
+
+  // Need to dynamically load error definitions
+  try {
+    const errorModule = await import(`@/constants/errors/${err.scope}.ts`)
+    registerError(err.scope, errorModule.default)
+    return translateErrorSync(err)
+  }
+  catch (error) {
+    console.error(error)
+    return $gettext(err?.message ?? 'Server error')
+  }
+}
+
+export async function handleApiError(err: CosyError, dedupe: MessageDedupe) {
+  dedupe.error(await translateError(err))
+}

+ 1 - 1
app/src/lib/http/interceptors.ts

@@ -153,7 +153,7 @@ export function setupResponseInterceptor() {
       }
 
       const err = error.response?.data as CosyError
-      handleApiError(err, dedupe)
+      await handleApiError(err, dedupe)
 
       return Promise.reject(error.response?.data)
     },

+ 5 - 0
app/src/views/dashboard/NginxDashBoard.vue

@@ -76,6 +76,11 @@ function connectSSE() {
       else {
         error.value = data.message || $gettext('Nginx is not running')
       }
+
+      if (data.error) {
+        error.value = data.error
+      }
+
       stubStatusEnabled.value = data.stub_status_enabled
     },
     onError: () => {

+ 0 - 31
internal/nginx/modules.go

@@ -1,7 +1,6 @@
 package nginx
 
 import (
-	"fmt"
 	"os"
 	"regexp"
 	"strings"
@@ -218,36 +217,6 @@ func getExpectedLoadModuleName(configureModuleName string) string {
 	return "ngx_" + normalized + "_module"
 }
 
-// GetModuleMapping returns a map showing the relationship between different module name formats.
-// This is useful for debugging and understanding how module names are processed.
-// Returns a map with normalized names as keys and mapping info as values.
-func GetModuleMapping() map[string]map[string]string {
-	modules := GetModules()
-	mapping := make(map[string]map[string]string)
-
-	modulesCacheLock.RLock()
-	defer modulesCacheLock.RUnlock()
-
-	// Use AllFromFront() to iterate through the ordered map
-	for normalizedName, module := range modules.AllFromFront() {
-		if module == nil {
-			continue
-		}
-
-		expectedLoadName := getExpectedLoadModuleName(normalizedName)
-
-		mapping[normalizedName] = map[string]string{
-			"normalized":           normalizedName,
-			"expected_load_module": expectedLoadName,
-			"dynamic":              fmt.Sprintf("%t", module.Dynamic),
-			"loaded":               fmt.Sprintf("%t", module.Loaded),
-			"params":               module.Params,
-		}
-	}
-
-	return mapping
-}
-
 // normalizeAddModuleName converts a module name from --add-module arguments
 // to a consistent format for internal use.
 // Examples:

+ 0 - 34
internal/nginx/modules_test.go

@@ -376,40 +376,6 @@ func TestExternalModuleDiscovery(t *testing.T) {
 	}
 }
 
-func TestGetModuleMapping(t *testing.T) {
-	// This test verifies that GetModuleMapping function works without errors
-	// Since it depends on nginx being available, we'll just test that it doesn't panic
-	defer func() {
-		if r := recover(); r != nil {
-			t.Errorf("GetModuleMapping panicked: %v", r)
-		}
-	}()
-
-	mapping := GetModuleMapping()
-
-	// The mapping should be a valid map (could be empty if nginx is not available)
-	if mapping == nil {
-		t.Error("GetModuleMapping returned nil")
-	}
-
-	t.Logf("GetModuleMapping returned %d entries", len(mapping))
-
-	// If there are entries, verify they have the expected structure
-	for moduleName, moduleInfo := range mapping {
-		if moduleInfo == nil {
-			t.Errorf("Module %s has nil info", moduleName)
-			continue
-		}
-
-		requiredFields := []string{"normalized", "expected_load_module", "dynamic", "loaded", "params"}
-		for _, field := range requiredFields {
-			if _, exists := moduleInfo[field]; !exists {
-				t.Errorf("Module %s missing field %s", moduleName, field)
-			}
-		}
-	}
-}
-
 func TestOpenRestyModuleParsing(t *testing.T) {
 	// Test case based on real OpenResty nginx -V output
 	openRestyOutput := `nginx version: openresty/1.25.3.1

+ 19 - 0
internal/performance/performance.go

@@ -15,6 +15,7 @@ type NginxPerformanceResponse struct {
 	StubStatusEnabled bool                 `json:"stub_status_enabled"`
 	Running           bool                 `json:"running"`
 	Info              NginxPerformanceInfo `json:"info"`
+	Error             string               `json:"error"`
 }
 
 func GetPerformanceData() NginxPerformanceResponse {
@@ -32,18 +33,36 @@ func GetPerformanceData() NginxPerformanceResponse {
 	stubStatusEnabled, statusInfo, err := GetStubStatusData()
 	if err != nil {
 		logger.Warn("Failed to get Nginx status:", err)
+		return NginxPerformanceResponse{
+			StubStatusEnabled: false,
+			Running:           running,
+			Info:              NginxPerformanceInfo{},
+			Error:             err.Error(),
+		}
 	}
 
 	// Get Nginx process information
 	processInfo, err := GetNginxProcessInfo()
 	if err != nil {
 		logger.Warn("Failed to get Nginx process info:", err)
+		return NginxPerformanceResponse{
+			StubStatusEnabled: stubStatusEnabled,
+			Running:           running,
+			Info:              NginxPerformanceInfo{},
+			Error:             err.Error(),
+		}
 	}
 
 	// Get Nginx config information
 	configInfo, err := GetNginxWorkerConfigInfo()
 	if err != nil {
 		logger.Warn("Failed to get Nginx config info:", err)
+		return NginxPerformanceResponse{
+			StubStatusEnabled: stubStatusEnabled,
+			Running:           running,
+			Info:              NginxPerformanceInfo{},
+			Error:             err.Error(),
+		}
 	}
 
 	// Ensure ProcessMode field is correctly passed