From 20f95d2f2be48014fafec9d88ef1e5759289b87a Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 24 Nov 2025 10:53:24 +0100 Subject: [PATCH] apimachinery: clarify ErrorHandler and provide helper for formatting It turned out that some downstream consumers converted their code after the API change in 1.32 (5a130d2b71e5d70cfff15087f4d521c6b68fb01e) by simply taking the error from the parameters, ignoring the rest. Not only is this capturing the problem incompletely, it will also crash for those HandleErrorWithContext/Logger calls which pass a nil error. Additional documentation might help a bit. The new ErrorToString helper can be used with or without errors.New to represent the full problem the way downstream consumers were traditionally expecting it. --- .../apimachinery/pkg/util/runtime/runtime.go | 32 +++++++++++++++++ .../pkg/util/runtime/runtime_test.go | 36 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go b/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go index 2a02957fd4b..38c1f7b03c4 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go @@ -17,14 +17,17 @@ limitations under the License. package runtime import ( + "bytes" "context" "fmt" "net/http" "runtime" + "strings" "sync" "time" "k8s.io/klog/v2" + "k8s.io/klog/v2/textlogger" ) var ( @@ -154,8 +157,37 @@ var ErrorHandlers = []ErrorHandler{ backoffError(1 * time.Millisecond), } +// ErrorHandler is called indirectly through [HandleError], [HandleErrorWithContext] or [HandleErrorWithLogger]. +// It is passed the same parameters that a structured logging backend needs to log a problem. +// It follows the semantic described for [HandleErrorWithContext] and [logr.Logger.Error]: +// - err is optional and may be nil +// - msg is string that describes the problem +// - keysAndValues contains additional information that varies between different occurrences of the problem +// +// [ErrorToString] can be used to convert these parameters into a single string, using the klog text output. type ErrorHandler func(ctx context.Context, err error, msg string, keysAndValues ...interface{}) +// ErrorToString takes the parameters passed to [ErrorHandler] and +// formats them as a string using the klog text output. +// +// If any of the values is a multi-line string, then the resulting +// string also uses line breaks and indention for the sake of readability. +// Does not include a trailing newline. +// +// Use errors.New if an error instead of a string is needed. +func ErrorToString(err error, msg string, keysAndValues ...interface{}) string { + var buffer bytes.Buffer + config := textlogger.NewConfig( + textlogger.Output(&buffer), + textlogger.WithHeader(false), + ) + logger := textlogger.NewLogger(config) + logger.Error(err, msg, keysAndValues...) + result := buffer.String() + result = strings.TrimSpace(result) + return result +} + // HandlerError is a method to invoke when a non-user facing piece of code cannot // return an error and needs to indicate it has been ignored. Invoking this method // is preferable to logging the error - the default behavior is to log but the diff --git a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go b/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go index 09e7bc104a9..5af48a7a806 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go @@ -311,3 +311,39 @@ func TestHandleError(t *testing.T) { }) } } + +func TestErrorToString(t *testing.T) { + for name, tt := range map[string]struct { + err error + msg string + kvs []any + expectString string + }{ + "simple": { + errors.New("some error"), + "Unhandled error", + nil, + `"Unhandled error" err="some error"`, + }, + "nil-error": { + nil, + "Some problem occurred", + nil, + `"Some problem occurred"`, + }, + "keys-and-values": { + errors.New("some error"), + "Some error occurred", + []any{"str", "foobar", "int", 1, "multiLine", "line 1\nline 2"}, + `"Some error occurred" err="some error" str="foobar" int=1 multiLine=< + line 1 + line 2 + >`, + }, + } { + t.Run(name, func(t *testing.T) { + actualString := ErrorToString(tt.err, tt.msg, tt.kvs...) + assert.Equal(t, tt.expectString, actualString) + }) + } +}