From 49dcf51fc95818dad32d0587829253c2d6169bca Mon Sep 17 00:00:00 2001 From: jinyisama Date: Sat, 11 Jul 2026 06:26:58 +0000 Subject: [PATCH] Improve error propagation for swallowed errors Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- internal/config/extension_config.go | 12 +++++- .../codex/auth_json_internal_test.go | 37 ++++++++++++++++ internal/extension/codex/catalog.go | 13 ++++-- internal/extension/metrics/plugin.go | 7 +++- internal/extension/visual/plugin.go | 8 +++- internal/service/app/app.go | 4 +- internal/service/server/dispatch.go | 10 ++++- .../service/server/dispatch_error_test.go | 42 +++++++++++++++++++ 8 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 internal/extension/codex/auth_json_internal_test.go create mode 100644 internal/service/server/dispatch_error_test.go diff --git a/internal/config/extension_config.go b/internal/config/extension_config.go index 1960afd8..828b1f36 100644 --- a/internal/config/extension_config.go +++ b/internal/config/extension_config.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "fmt" + "log/slog" "gopkg.in/yaml.v3" ) @@ -239,7 +240,14 @@ func decodeTypedExtensionConfig(spec ExtensionConfigSpec, raw map[string]any) an if typed == nil { return cloneAnyMap(raw) } - data, _ := json.Marshal(raw) - _ = json.Unmarshal(data, typed) + data, err := json.Marshal(raw) + if err != nil { + slog.Warn("扩展配置序列化失败，回退到原始配置", "extension", spec.Name, "error", err) + return cloneAnyMap(raw) + } + if err := json.Unmarshal(data, typed); err != nil { + slog.Warn("扩展配置解码失败，回退到原始配置", "extension", spec.Name, "error", err) + return cloneAnyMap(raw) + } return typed } diff --git a/internal/extension/codex/auth_json_internal_test.go b/internal/extension/codex/auth_json_internal_test.go new file mode 100644 index 00000000..cadff95a --- /dev/null +++ b/internal/extension/codex/auth_json_internal_test.go @@ -0,0 +1,37 @@ +package codex + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestWriteAuthJSONWritesValidContent(t *testing.T) { + path := filepath.Join(t.TempDir(), "nested", "auth.json") + if err := writeAuthJSON(path, "sk-test-token"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read auth.json: %v", err) + } + var parsed map[string]string + if err := json.Unmarshal(data, &parsed); err != nil { + t.Fatalf("auth.json is not valid JSON: %v", err) + } + if parsed["openai_api_key"] != "sk-test-token" { + t.Fatalf("unexpected token: %q", parsed["openai_api_key"]) + } +} + +func TestWriteAuthJSONReturnsErrorOnUnwritablePath(t *testing.T) { + // A path whose parent is an existing regular file cannot be created. + file := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(file, []byte("x"), 0600); err != nil { + t.Fatalf("setup: %v", err) + } + if err := writeAuthJSON(filepath.Join(file, "auth.json"), "tok"); err == nil { + t.Fatal("expected error when parent path is a file, got nil") + } +} diff --git a/internal/extension/codex/catalog.go b/internal/extension/codex/catalog.go index 75ab8823..0a378615 100644 --- a/internal/extension/codex/catalog.go +++ b/internal/extension/codex/catalog.go @@ -562,7 +562,7 @@ func GenerateConfigToml(output io.Writer, modelAlias string, baseURL string, cod // writeAuthJSON writes the API key into Codex's auth.json so that model_providers // using requires_openai_auth can find the bearer token. -func writeAuthJSON(path, token string) error { +func writeAuthJSON(path, token string) (err error) { dir := filepath.Dir(path) if err := os.MkdirAll(dir, 0755); err != nil { return err @@ -571,6 +571,13 @@ func writeAuthJSON(path, token string) error { if err != nil { return err } - defer f.Close() - return json.NewEncoder(f).Encode(map[string]string{"openai_api_key": token}) + defer func() { + if closeErr := f.Close(); closeErr != nil && err == nil { + err = fmt.Errorf("close %s: %w", path, closeErr) + } + }() + if err := json.NewEncoder(f).Encode(map[string]string{"openai_api_key": token}); err != nil { + return fmt.Errorf("encode auth.json: %w", err) + } + return nil } diff --git a/internal/extension/metrics/plugin.go b/internal/extension/metrics/plugin.go index df8ecc5a..74a66332 100644 --- a/internal/extension/metrics/plugin.go +++ b/internal/extension/metrics/plugin.go @@ -201,8 +201,11 @@ func (p *Plugin) handleQuery(w http.ResponseWriter, r *http.Request) { var cfg *Config if setting, ok := p.pluginCfg.Extensions[PluginName]; ok && len(setting.RawConfig) > 0 { data, err := json.Marshal(setting.RawConfig) - if err == nil { - _ = json.Unmarshal(data, &cfg) + if err != nil { + slog.Warn("metrics 配置序列化失败，使用默认值", "error", err) + } else if err := json.Unmarshal(data, &cfg); err != nil { + slog.Warn("metrics 配置解码失败，使用默认值", "error", err) + cfg = nil } } defaultLimit := 100 diff --git a/internal/extension/visual/plugin.go b/internal/extension/visual/plugin.go index c218fb9e..0c0a7917 100644 --- a/internal/extension/visual/plugin.go +++ b/internal/extension/visual/plugin.go @@ -3,6 +3,7 @@ package visual import ( "encoding/json" "fmt" + "log/slog" "strings" "moonbridge/internal/config" @@ -98,8 +99,11 @@ func ConfigForModel(pluginCfg config.PluginConfig, modelAlias string) (Config, b var cfg *Config if setting, ok := pluginCfg.Extensions[PluginName]; ok && len(setting.RawConfig) > 0 { data, err := json.Marshal(setting.RawConfig) - if err == nil { - _ = json.Unmarshal(data, &cfg) + if err != nil { + slog.Warn("visual 配置序列化失败，使用默认值", "error", err) + } else if err := json.Unmarshal(data, &cfg); err != nil { + slog.Warn("visual 配置解码失败，使用默认值", "error", err) + cfg = nil } } if cfg == nil { diff --git a/internal/service/app/app.go b/internal/service/app/app.go index 5e6e2adb..d2af2939 100644 --- a/internal/service/app/app.go +++ b/internal/service/app/app.go @@ -650,7 +650,9 @@ func runHTTPServer(ctx context.Context, addr string, handler http.Handler, error httpServer := &http.Server{Addr: addr, Handler: handler} defer func() { if closer, ok := handler.(io.Closer); ok { - _ = closer.Close() + if err := closer.Close(); err != nil { + slog.Error("关闭 HTTP handler 失败", "error", err) + } } }() errCh := make(chan error, 1) diff --git a/internal/service/server/dispatch.go b/internal/service/server/dispatch.go index 4980b6ba..c081b70f 100644 --- a/internal/service/server/dispatch.go +++ b/internal/service/server/dispatch.go @@ -249,7 +249,9 @@ func traceError(stage string, err error) map[string]string { func writeJSON(writer http.ResponseWriter, status int, payload any) { writer.Header().Set("Content-Type", "application/json") writer.WriteHeader(status) - _ = json.NewEncoder(writer).Encode(payload) + if err := json.NewEncoder(writer).Encode(payload); err != nil { + slog.Warn("写入 JSON 响应失败", "status", status, "error", err) + } } func writeOpenAIError(writer http.ResponseWriter, status int, payload openai.ErrorResponse) { writeJSON(writer, status, payload) @@ -259,7 +261,11 @@ func writeSSE(writer http.ResponseWriter, event openai.StreamEvent) error { if event.Data == nil { payload = []byte("{}") } else { - payload, _ = json.Marshal(event.Data) + marshaled, err := json.Marshal(event.Data) + if err != nil { + return fmt.Errorf("marshal SSE event %q: %w", event.Event, err) + } + payload = marshaled } if _, err := writer.Write([]byte("event: " + event.Event + "\n")); err != nil { return err diff --git a/internal/service/server/dispatch_error_test.go b/internal/service/server/dispatch_error_test.go new file mode 100644 index 00000000..1ee35fb4 --- /dev/null +++ b/internal/service/server/dispatch_error_test.go @@ -0,0 +1,42 @@ +package server + +import ( + "net/http/httptest" + "strings" + "testing" + + "moonbridge/internal/protocol/openai" +) + +// writeSSE must propagate marshalling failures instead of silently emitting a +// truncated/empty data frame. +func TestWriteSSEPropagatesMarshalError(t *testing.T) { + rec := httptest.NewRecorder() + // channels are not JSON-serializable, forcing json.Marshal to fail. + event := openai.StreamEvent{Event: "response.output_text.delta", Data: make(chan int)} + + err := writeSSE(rec, event) + if err == nil { + t.Fatal("expected error when event data cannot be marshalled, got nil") + } + if !strings.Contains(err.Error(), "marshal SSE event") { + t.Fatalf("expected marshal error, got %q", err.Error()) + } +} + +// writeSSE serializes a well-formed event into the expected SSE frame. +func TestWriteSSEWritesFrame(t *testing.T) { + rec := httptest.NewRecorder() + event := openai.StreamEvent{Event: "ping", Data: map[string]string{"k": "v"}} + + if err := writeSSE(rec, event); err != nil { + t.Fatalf("unexpected error: %v", err) + } + body := rec.Body.String() + if !strings.Contains(body, "event: ping\n") { + t.Fatalf("missing event line in %q", body) + } + if !strings.Contains(body, `data: {"k":"v"}`) { + t.Fatalf("missing data line in %q", body) + } +}