Browse Source

ipn/localapi: stop logging "broken pipe" errors (#18487)

The Tailscale CLI has some methods to watch the IPN bus for
messages, say, the current netmap (`tailscale debug netmap`).
The Tailscale daemon supports this using a streaming HTTP
response. Sometimes, the client can close its connection
abruptly -- due to an interruption, or in the case of `debug netmap`,
intentionally after consuming one message.

If the server daemon is writing a response as the client closes
its end of the socket, the daemon typically encounters a "broken pipe"
error. The "Watch IPN Bus" handler currently logs such errors after
they're propagated by a JSON encoding/writer helper.

Since the Tailscale CLI nominally closes its socket with the daemon
in this slightly ungraceful way (viz. `debug netmap`), stop logging
these broken pipe errors as far as possible. This will help avoid
confounding users when they scan backend logs.

Updates #18477

Signed-off-by: Amal Bansode <[email protected]>
Amal Bansode 1 month ago
parent
commit
6de5b01e04

+ 4 - 1
ipn/localapi/localapi.go

@@ -35,6 +35,7 @@ import (
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/logtail"
+	"tailscale.com/net/neterror"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/netutil"
 	"tailscale.com/tailcfg"
@@ -913,7 +914,9 @@ func (h *Handler) serveWatchIPNBus(w http.ResponseWriter, r *http.Request) {
 	h.b.WatchNotificationsAs(ctx, h.Actor, mask, f.Flush, func(roNotify *ipn.Notify) (keepGoing bool) {
 		err := enc.Encode(roNotify)
 		if err != nil {
-			h.logf("json.Encode: %v", err)
+			if !neterror.IsClosedPipeError(err) {
+				h.logf("json.Encode: %v", err)
+			}
 			return false
 		}
 		f.Flush()

+ 20 - 0
net/neterror/neterror_js.go

@@ -0,0 +1,20 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build js || wasip1 || wasm
+
+package neterror
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+)
+
+// Reports whether err resulted from reading or writing to a closed or broken pipe.
+func IsClosedPipeError(err error) bool {
+	// Libraries may also return root errors like fs.ErrClosed/io.ErrClosedPipe
+	// due to a closed socket.
+	return errors.Is(err, fs.ErrClosed) ||
+		errors.Is(err, io.ErrClosedPipe)
+}

+ 24 - 0
net/neterror/neterror_plan9.go

@@ -0,0 +1,24 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build plan9
+
+package neterror
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+	"strings"
+)
+
+// Reports whether err resulted from reading or writing to a closed or broken pipe.
+func IsClosedPipeError(err error) bool {
+	// Libraries may also return root errors like fs.ErrClosed/io.ErrClosedPipe
+	// due to a closed socket.
+	// For a raw syscall error, check for error string containing "closed pipe",
+	// per the note set by the system: https://9p.io/magic/man2html/2/pipe
+	return errors.Is(err, fs.ErrClosed) ||
+		errors.Is(err, io.ErrClosedPipe) ||
+		strings.Contains(err.Error(), "closed pipe")
+}

+ 32 - 0
net/neterror/neterror_posix.go

@@ -0,0 +1,32 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !plan9 && !js && !wasip1 && !wasm
+
+package neterror
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+	"runtime"
+	"syscall"
+)
+
+// Reports whether err resulted from reading or writing to a closed or broken pipe.
+func IsClosedPipeError(err error) bool {
+	// 232 is Windows error code ERROR_NO_DATA, "The pipe is being closed".
+	if runtime.GOOS == "windows" && errors.Is(err, syscall.Errno(232)) {
+		return true
+	}
+
+	// EPIPE/ENOTCONN are common errors when a send fails due to a closed
+	// socket. There is some platform and version inconsistency in which
+	// error is returned, but the meaning is the same.
+	// Libraries may also return root errors like fs.ErrClosed/io.ErrClosedPipe
+	// due to a closed socket.
+	return errors.Is(err, syscall.EPIPE) ||
+		errors.Is(err, syscall.ENOTCONN) ||
+		errors.Is(err, fs.ErrClosed) ||
+		errors.Is(err, io.ErrClosedPipe)
+}

+ 3 - 1
wgengine/magicsock/magicsock_notplan9.go

@@ -8,6 +8,8 @@ package magicsock
 import (
 	"errors"
 	"syscall"
+
+	"tailscale.com/net/neterror"
 )
 
 // shouldRebind returns if the error is one that is known to be healed by a
@@ -17,7 +19,7 @@ func shouldRebind(err error) (ok bool, reason string) {
 	// EPIPE/ENOTCONN are common errors when a send fails due to a closed
 	// socket. There is some platform and version inconsistency in which
 	// error is returned, but the meaning is the same.
-	case errors.Is(err, syscall.EPIPE), errors.Is(err, syscall.ENOTCONN):
+	case neterror.IsClosedPipeError(err):
 		return true, "broken-pipe"
 
 	// EPERM is typically caused by EDR software, and has been observed to be