fix: resolve LogValuer before processing in attribute helpers#39
Conversation
Call v.Resolve() at the start of AnyValueToString, anyValueToStringValue, and ValueToFloat64, and use Resolve() when extracting error values in ReplaceAttrFunctions. Fixes incorrect dispatch when values are wrapped in a LogValuer.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
==========================================
+ Coverage 96.15% 96.18% +0.02%
==========================================
Files 4 4
Lines 260 262 +2
==========================================
+ Hits 250 252 +2
Misses 9 9
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the attribute/value helper functions to call slog.Value.Resolve() before inspecting or formatting values, improving handling of slog.LogValuer-wrapped values during conversion and error replacement.
Changes:
- Resolve
slog.LogValuervalues early inAttrToValue,AnyValueToString, andValueToString. - Use
Resolve()when extractingerrorvalues inReplaceErrorto better support LogValuer-wrapped errors.
Comments suppressed due to low confidence (1)
attributes.go:165
- The new
v = v.Resolve()behavior inAnyValueToString/ValueToString/AttrToValuechanges howslog.LogValuervalues are rendered (and is the core purpose of this PR), but existing tests don’t cover LogValuer inputs for these helpers. Add test cases that passslog.Any("k", stubLogValuer)(and a LogValuer that returns a non-group liketestMaskedLogValuer) through these helpers to ensure the resolved output is used and no raw LogValuer structs leak into logs.
func AnyValueToString(v slog.Value) string {
v = v.Resolve()
if tm, ok := v.Any().(encoding.TextMarshaler); ok {
data, err := tm.MarshalText()
if err != nil {
return ""
}
return string(data)
}
return fmt.Sprintf("%+v", v.Any())
}
func AttrsToString(attrs ...slog.Attr) map[string]string {
output := make(map[string]string, len(attrs))
for i := range attrs {
attr := attrs[i]
k, v := attr.Key, attr.Value
output[k] = ValueToString(v)
}
return output
}
func ValueToString(v slog.Value) string {
v = v.Resolve()
switch v.Kind() {
case slog.KindAny, slog.KindLogValuer, slog.KindGroup:
return AnyValueToString(v)
case slog.KindInt64, slog.KindUint64, slog.KindFloat64, slog.KindString, slog.KindBool, slog.KindDuration:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for i := range errorKeys { | ||
| if a.Key == errorKeys[i] { | ||
| if err, ok := a.Value.Any().(error); ok { | ||
| if err, ok := a.Value.Resolve().Any().(error); ok { | ||
| return slog.Any(a.Key, FormatError(err)) | ||
| } |
There was a problem hiding this comment.
ReplaceError now checks a.Value.Resolve().Any().(error). This breaks replacement for values where the attribute itself is an error that also implements slog.LogValuer: Resolve() converts it to the result of LogValue() (e.g., a string/group), so the type assertion to error will fail and the error won’t be formatted. Consider checking a.Value.Any() for error first (preserving LogValuer errors), and only falling back to Resolve().Any() if the direct assertion fails (to support wrapper LogValuers).
| func AttrToValue(attr slog.Attr) (string, any) { | ||
| k := attr.Key | ||
| v := attr.Value | ||
| kind := v.Kind() | ||
| v = v.Resolve() | ||
|
|
||
| switch kind { | ||
| switch v.Kind() { |
There was a problem hiding this comment.
The PR description references anyValueToStringValue, ValueToFloat64, and "ReplaceAttrFunctions", but those functions don’t exist in this repo and the changes here also update ValueToString/AttrToValue. Please update the PR description to reflect the actual modified helpers so reviewers/users can match the intent to the diff.
Call v.Resolve() at the start of AnyValueToString, anyValueToStringValue,
and ValueToFloat64, and use Resolve() when extracting error values in
ReplaceAttrFunctions. Fixes incorrect dispatch when values are wrapped
in a LogValuer.