From f2ed2a836b44518c57ea38827b2f9f2740ea8df6 Mon Sep 17 00:00:00 2001 From: adababys <8872412+adababys@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:32:49 +0800 Subject: [PATCH] fix(proxy): distinguish upstream failures from routing failures in proxy ErrorHandler The proxy ErrorHandler returned the same "Failed to route request to sandbox" message for two unrelated cases: a genuine routing failure (destination not resolved, 500) and an upstream/transport failure after the destination was already resolved (502). Operators reading the logs could not tell a real routing problem apart from a client that dropped a large upload or an envd that answered 5xx. - The 502 path now returns a distinct message ("Failed to reach sandbox (upstream connection error)"), so the 500 routing failures and the 502 upstream failures are unambiguous in both logs and client responses. - Client-caused disconnects (context.Canceled, ECONNRESET, io.EOF) are logged at Warn instead of Error. A weak/CGNAT link dropping a large POST /files upload, or an SDK giving up mid-request, is not a server fault and should not drown real upstream errors in ERROR-level noise. --- packages/shared/pkg/proxy/pool/client.go | 29 ++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/shared/pkg/proxy/pool/client.go b/packages/shared/pkg/proxy/pool/client.go index 3fad1a73d9..ed2e0eacee 100644 --- a/packages/shared/pkg/proxy/pool/client.go +++ b/packages/shared/pkg/proxy/pool/client.go @@ -4,11 +4,13 @@ import ( "context" "crypto/tls" "errors" + "io" "log" "net" "net/http" "net/http/httputil" "sync/atomic" + "syscall" "time" "go.uber.org/zap" @@ -153,14 +155,20 @@ func newProxyClient( } if err != nil { - if t.SandboxPort == uint64(consts.DefaultEnvdServerPort) { - if errors.Is(err, context.Canceled) { - t.RequestLogger.Warn(ctx, "sandbox request canceled by client", zap.Error(err)) - } else { - t.RequestLogger.Error(ctx, "sandbox error handler called", zap.Error(err)) - } - } else { - t.RequestLogger.Warn(ctx, "sandbox error handler called", zap.Error(err)) + // Client-caused disconnects (cancel, connection reset, EOF) are not + // server faults: an SDK/browser that gives up mid-request, or a weak + // (CGNAT/mobile) link that drops a large upload, surfaces here. Log + // them at Warn so they do not drown real upstream faults in ERROR. + clientGone := errors.Is(err, context.Canceled) || + errors.Is(err, syscall.ECONNRESET) || + errors.Is(err, io.EOF) + switch { + case clientGone: + t.RequestLogger.Warn(ctx, "sandbox request aborted by client", zap.Error(err)) + case t.SandboxPort == uint64(consts.DefaultEnvdServerPort): + t.RequestLogger.Error(ctx, "sandbox upstream error", zap.Error(err)) + default: + t.RequestLogger.Warn(ctx, "sandbox upstream error", zap.Error(err)) } } @@ -179,7 +187,10 @@ func newProxyClient( return } - cors.Error(w, "Failed to route request to sandbox", http.StatusBadGateway) + // Destination resolved; this is an upstream/transport failure reaching + // the sandbox, not a routing failure. A distinct message and 502 let + // logs and clients tell it apart from the 500 routing failures above. + cors.Error(w, "Failed to reach sandbox (upstream connection error)", http.StatusBadGateway) }, ModifyResponse: func(r *http.Response) error { ctx := r.Request.Context()