From 6d7249ab543027cfa1eaf9b3dc5f1d1abe6282da 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 | 41 +++++++++++++++++++ .../pkg/util/runtime/runtime_test.go | 36 ++++++++++++++++ 2 files changed, 77 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 cc09bdbc436..cb8e0949cea 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 ( @@ -155,8 +158,46 @@ 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 + + // There's no API for suppressing the header, so we let it fill with constant + // information and then just strip it. Will become simpler should + // https://github.com/kubernetes/klog/issues/429 get implemented. + config := textlogger.NewConfig( + textlogger.Output(&buffer), + textlogger.Backtrace(func(int) (string, int) { return "", 0 }), + textlogger.FixedTime(time.Time{}), + ) + logger := textlogger.NewLogger(config) + logger.Error(err, msg, keysAndValues...) + result := buffer.String() + result = strings.TrimSpace(result) + index := strings.Index(result, "] ") + if index > 0 { + return result[index+2:] + } + 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) + }) + } +}