From 3dbe6954d7ba0139fc736d55568edd7904eefba0 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 10 Aug 2026 12:59:58 +0000 Subject: [PATCH 1/2] Add a failing test pinning the discarded slog attributes Every handler in this package accepts slog attributes and then throws them away: Handle never reads record.Attrs, and WithAttrs and WithGroup return the receiver unchanged in ConsoleHandler, JSONHandler and WebhookHandler alike. So slog.Info("casting", "device", d, "file", f) emits the message and silently loses both fields, which makes properly structured logging carry less information than the interpolated log.Printf calls it replaces. The test asserts on the bytes the handlers actually write - os.Stdout for the console and JSON handlers, the posted body for the webhook handler - rather than on internal state, because the output is where the loss is observable. It covers record attributes, WithAttrs accumulation, WithAttrs not mutating its receiver so sibling loggers cannot leak attributes into each other, WithGroup qualification, slog.Group nesting, and LogValuer resolution, for each handler and through MultiplexHandler. It also pins the properties a fix must not get wrong on the way past. A value the caller logged is read and never written to, even when a group later claims the same key - for maps and for slices, through the record path and the derived-logger path, in all three handlers. A duration is a number of nanoseconds in the json payload and the readable "3s" form on the console. A console key is quoted on the same terms as a console value, and as one token including its group prefix, so that the "=" that separates the pair is always the first one outside quotes: a key of "a=b" reads as "a=b"=v rather than the ambiguous a=b=v. The four slog.Handler contract edge cases hold: an empty Attr is ignored, an empty group is elided with its key, a group with an empty key is inlined, and WithGroup("") is a no-op. And the two rules that follow from the json payload being an object hold too: the record's own field names win a collision, and a repeated key keeps its last value - except where both are groups, which merge - while the console line keeps both. This commit adds only the test, and it fails. The fix follows. Refs: https://git.eeqj.de/sneak/simplelog/issues/19 --- attrs_test.go | 905 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 905 insertions(+) create mode 100644 attrs_test.go diff --git a/attrs_test.go b/attrs_test.go new file mode 100644 index 0000000..3ee5bae --- /dev/null +++ b/attrs_test.go @@ -0,0 +1,905 @@ +package simplelog + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "reflect" + "strings" + "testing" + "time" +) + +// These tests assert on the bytes the handlers actually emit, because that is +// where the defect lives: the handlers accept attributes and then throw them +// away, so nothing short of reading the output proves they survived. + +// captureStdout redirects os.Stdout for the duration of fn and returns what was +// written to it. Both the console and the JSON handler write to os.Stdout, so +// this is the only way to see their real output. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + + original := os.Stdout + os.Stdout = w + + collected := make(chan string, 1) + go func() { + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + collected <- buf.String() + }() + + defer func() { + os.Stdout = original + _ = r.Close() + }() + + fn() + + os.Stdout = original + if err := w.Close(); err != nil { + t.Fatalf("close pipe writer: %v", err) + } + + return <-collected +} + +// testRecord builds an INFO record carrying the given attributes. +func testRecord(message string, attrs ...slog.Attr) slog.Record { + record := slog.NewRecord(time.Now(), slog.LevelInfo, message, 0) + record.AddAttrs(attrs...) + return record +} + +// decodeLine parses a single line of JSON handler output. +func decodeLine(t *testing.T, output string) map[string]any { + t.Helper() + + line := strings.TrimSpace(output) + if line == "" { + t.Fatal("handler emitted no output") + } + + var decoded map[string]any + if err := json.Unmarshal([]byte(line), &decoded); err != nil { + t.Fatalf("output is not valid JSON: %v\noutput: %s", err, line) + } + return decoded +} + +// wantField asserts that a decoded JSON object has key with the given value. +func wantField(t *testing.T, decoded map[string]any, key string, want any) { + t.Helper() + + got, ok := decoded[key] + if !ok { + t.Fatalf("field %q missing from output: %v", key, decoded) + } + if got != want { + t.Fatalf("field %q = %v, want %v", key, got, want) + } +} + +// wantGroup asserts that a decoded JSON object has key holding a nested object. +func wantGroup(t *testing.T, decoded map[string]any, key string) map[string]any { + t.Helper() + + got, ok := decoded[key] + if !ok { + t.Fatalf("group %q missing from output: %v", key, decoded) + } + group, ok := got.(map[string]any) + if !ok { + t.Fatalf("field %q = %v, want a nested object", key, got) + } + return group +} + +// wantNoField asserts that a decoded JSON object does not carry key. +func wantNoField(t *testing.T, decoded map[string]any, key string) { + t.Helper() + + if got, present := decoded[key]; present { + t.Fatalf("field %q unexpectedly present as %v: %v", key, got, decoded) + } +} + +// wantUnchanged asserts that a value the caller still owns was not written to +// by the handler. Logging must read what it is handed, never modify it. +func wantUnchanged(t *testing.T, what string, got, want any) { + t.Helper() + + if !reflect.DeepEqual(got, want) { + t.Fatalf("handler mutated the caller's %s: got %v, want %v", what, got, want) + } +} + +// wantContains asserts that console output contains a fragment. +func wantContains(t *testing.T, output, fragment string) { + t.Helper() + + if !strings.Contains(output, fragment) { + t.Fatalf("output does not contain %q\noutput: %s", fragment, output) + } +} + +// wantNotContains asserts that console output does not contain a fragment. +func wantNotContains(t *testing.T, output, fragment string) { + t.Helper() + + if strings.Contains(output, fragment) { + t.Fatalf("output unexpectedly contains %q\noutput: %s", fragment, output) + } +} + +// castTarget is a slog.LogValuer: the handler must resolve it rather than +// serialising the struct itself. +type castTarget struct { + id string +} + +func (c castTarget) LogValue() slog.Value { + return slog.StringValue(c.id) +} + +var _ slog.LogValuer = castTarget{} + +func TestJSONHandlerEmitsRecordAttrs(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord( + "casting", + slog.String("device", "livingroom"), + slog.String("file", "movie.mp4"), + slog.Int("attempt", 3), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + wantField(t, decoded, "Message", "casting") + wantField(t, decoded, "device", "livingroom") + wantField(t, decoded, "file", "movie.mp4") + wantField(t, decoded, "attempt", float64(3)) +} + +func TestJSONHandlerWithAttrsAccumulates(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler(). + WithAttrs([]slog.Attr{slog.String("service", "cattbox")}). + WithAttrs([]slog.Attr{slog.String("component", "caster")}) + record := testRecord("casting", slog.String("device", "livingroom")) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + wantField(t, decoded, "service", "cattbox") + wantField(t, decoded, "component", "caster") + wantField(t, decoded, "device", "livingroom") +} + +func TestJSONHandlerWithAttrsDoesNotMutateReceiver(t *testing.T) { + parent := NewJSONHandler() + first := parent.WithAttrs([]slog.Attr{slog.String("worker", "first")}) + second := parent.WithAttrs([]slog.Attr{slog.String("worker", "second")}) + + firstOutput := captureStdout(t, func() { + if err := first.Handle(context.Background(), testRecord("work")); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + secondOutput := captureStdout(t, func() { + if err := second.Handle(context.Background(), testRecord("work")); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + parentOutput := captureStdout(t, func() { + if err := parent.Handle(context.Background(), testRecord("work")); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantField(t, decodeLine(t, firstOutput), "worker", "first") + wantField(t, decodeLine(t, secondOutput), "worker", "second") + + if _, present := decodeLine(t, parentOutput)["worker"]; present { + t.Fatalf("parent handler leaked an attribute from a derived handler: %s", parentOutput) + } +} + +func TestJSONHandlerWithGroupNestsAttrs(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler(). + WithGroup("cast"). + WithAttrs([]slog.Attr{slog.String("device", "livingroom")}) + record := testRecord("casting", slog.String("file", "movie.mp4")) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + wantField(t, decoded, "Message", "casting") + + group := wantGroup(t, decoded, "cast") + wantField(t, group, "device", "livingroom") + wantField(t, group, "file", "movie.mp4") +} + +func TestJSONHandlerResolvesValues(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord( + "cast failed", + slog.Group("request", slog.Int("status", 502), slog.String("method", "POST")), + slog.Any("target", castTarget{id: "chromecast-7"}), + slog.Any("error", errors.New("connection refused")), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + wantField(t, decoded, "target", "chromecast-7") + wantField(t, decoded, "error", "connection refused") + + group := wantGroup(t, decoded, "request") + wantField(t, group, "status", float64(502)) + wantField(t, group, "method", "POST") +} + +func TestConsoleHandlerEmitsRecordAttrs(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler() + record := testRecord( + "casting", + slog.String("device", "livingroom"), + slog.String("file", "movie.mp4"), + slog.Int("attempt", 3), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, "casting") + wantContains(t, output, "device=livingroom") + wantContains(t, output, "file=movie.mp4") + wantContains(t, output, "attempt=3") +} + +func TestConsoleHandlerQuotesValuesNeedingIt(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler() + record := testRecord("casting", slog.String("file", "The Movie.mp4")) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, `file="The Movie.mp4"`) +} + +// TestConsoleHandlerQuotesKeysNeedingIt pins the key side of the same rule. +// A key is quoted on the same terms as a value, and as one token including +// its group prefix, so that the "=" separating the pair is always the first +// one outside quotes. Every case here was compared against +// slog.NewTextHandler, which renders each of them identically. +func TestConsoleHandlerQuotesKeysNeedingIt(t *testing.T) { + tests := []struct { + name string + build func() slog.Handler + attrs []slog.Attr + want string + notWant string + }{ + { + name: "an ordinary key is left bare", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{slog.String("device", "livingroom")}, + want: " device=livingroom", + notWant: `"device"`, + }, + { + name: "a key containing an equals sign is quoted", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{slog.String("a=b", "v2")}, + want: ` "a=b"=v2`, + notWant: " a=b=v2", + }, + { + name: "a key containing a space is quoted", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{slog.String("my key", "v")}, + want: ` "my key"=v`, + notWant: " my key=v", + }, + { + name: "a key containing a quote is escaped", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{slog.String(`he"llo`, "v")}, + want: ` "he\"llo"=v`, + notWant: ` he"llo=v`, + }, + { + name: "a printable non-ascii key is left bare", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{slog.String("キー", "v")}, + want: " キー=v", + notWant: `"キー"`, + }, + { + name: "a group prefix is quoted together with its key", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{ + slog.Group("grp", slog.String("a=b", "v")), + }, + want: ` "grp.a=b"=v`, + notWant: " grp.a=b=v", + }, + { + name: "a WithGroup prefix needing quotes quotes the whole key", + build: func() slog.Handler { + return NewConsoleHandler().WithGroup("my grp") + }, + attrs: []slog.Attr{slog.String("k", "v")}, + want: ` "my grp.k"=v`, + notWant: " my grp.k=v", + }, + { + name: "an empty key is quoted rather than left as a gap", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{slog.String("", "v")}, + want: ` ""=v`, + notWant: " =v", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + output := captureStdout(t, func() { + handler := test.build() + record := testRecord("casting", test.attrs...) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, test.want) + wantNotContains(t, output, test.notWant) + }) + } +} + +func TestConsoleHandlerWithAttrsAccumulates(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler(). + WithAttrs([]slog.Attr{slog.String("service", "cattbox")}). + WithAttrs([]slog.Attr{slog.String("component", "caster")}) + record := testRecord("casting", slog.String("device", "livingroom")) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, "service=cattbox") + wantContains(t, output, "component=caster") + wantContains(t, output, "device=livingroom") +} + +func TestConsoleHandlerWithAttrsDoesNotMutateReceiver(t *testing.T) { + parent := NewConsoleHandler() + first := parent.WithAttrs([]slog.Attr{slog.String("worker", "first")}) + second := parent.WithAttrs([]slog.Attr{slog.String("worker", "second")}) + + firstOutput := captureStdout(t, func() { + if err := first.Handle(context.Background(), testRecord("work")); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + secondOutput := captureStdout(t, func() { + if err := second.Handle(context.Background(), testRecord("work")); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + parentOutput := captureStdout(t, func() { + if err := parent.Handle(context.Background(), testRecord("work")); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, firstOutput, "worker=first") + wantNotContains(t, firstOutput, "worker=second") + wantContains(t, secondOutput, "worker=second") + wantNotContains(t, secondOutput, "worker=first") + wantNotContains(t, parentOutput, "worker=") +} + +func TestConsoleHandlerWithGroupQualifiesAttrs(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler(). + WithGroup("cast"). + WithAttrs([]slog.Attr{slog.String("device", "livingroom")}) + record := testRecord("casting", slog.String("file", "movie.mp4")) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, "cast.device=livingroom") + wantContains(t, output, "cast.file=movie.mp4") +} + +func TestConsoleHandlerResolvesValues(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler() + record := testRecord( + "cast failed", + slog.Group("request", slog.Int("status", 502)), + slog.Any("target", castTarget{id: "chromecast-7"}), + slog.Any("error", errors.New("connection refused")), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, "request.status=502") + wantContains(t, output, "target=chromecast-7") + wantContains(t, output, `error="connection refused"`) +} + +func TestWebhookHandlerEmitsAttrs(t *testing.T) { + bodies := make(chan []byte, 1) + server := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read webhook body: %v", err) + } + bodies <- body + w.WriteHeader(http.StatusOK) + }, + )) + defer server.Close() + + handler, err := NewWebhookHandler(server.URL) + if err != nil { + t.Fatalf("NewWebhookHandler: %v", err) + } + + withAttrs := handler.WithAttrs([]slog.Attr{slog.String("service", "cattbox")}) + record := testRecord("casting", slog.String("device", "livingroom")) + if err := withAttrs.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + + var body []byte + select { + case body = <-bodies: + case <-time.After(5 * time.Second): + t.Fatal("webhook handler posted nothing") + } + + decoded := decodeLine(t, string(body)) + wantField(t, decoded, "service", "cattbox") + wantField(t, decoded, "device", "livingroom") +} + +// TestMultiplexHandlerPassesAttrsThrough guards the composite handler that +// package init installs: attributes must survive the multiplex too. +func TestMultiplexHandlerPassesAttrsThrough(t *testing.T) { + output := captureStdout(t, func() { + handler := (&MultiplexHandler{handlers: []ExtendedHandler{NewJSONHandler()}}). + WithAttrs([]slog.Attr{slog.String("service", "cattbox")}) + record := testRecord("casting", slog.String("device", "livingroom")) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + wantField(t, decoded, "service", "cattbox") + wantField(t, decoded, "device", "livingroom") +} + +// A logging library must read the values it is handed and never write to them. +// The dangerous shape is a group that shares a key with something the caller +// logged by reference: merging the group's members into whatever already sits +// at that key would reach straight back into the caller's own data structure. +// The next four tests hold every handler to that, through both the record path +// and the derived-logger path. + +func TestJSONHandlerDoesNotMutateCallerMap(t *testing.T) { + caller := map[string]any{"mine": "untouched"} + + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord( + "casting", + slog.Any("g", caller), + slog.Group("g", slog.Int("injected", 1)), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantUnchanged(t, "map", caller, map[string]any{"mine": "untouched"}) + + // The later attribute wins the key outright, as any repeated key does. + group := wantGroup(t, decodeLine(t, output), "g") + wantField(t, group, "injected", float64(1)) + wantNoField(t, group, "mine") +} + +func TestJSONHandlerDoesNotMutateCallerMapThroughWithAttrs(t *testing.T) { + caller := map[string]any{"id": "req-1"} + + output := captureStdout(t, func() { + handler := NewJSONHandler(). + WithAttrs([]slog.Attr{slog.Any("req", caller)}). + WithGroup("req") + record := testRecord("cast failed", slog.Int("status", 502)) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantUnchanged(t, "map", caller, map[string]any{"id": "req-1"}) + + group := wantGroup(t, decodeLine(t, output), "req") + wantField(t, group, "status", float64(502)) + wantNoField(t, group, "id") +} + +// TestJSONHandlerDoesNotMutateCallerSlice is the slice form of the same +// hazard: a caller's slice is also handed over by reference, and appending +// into its spare capacity would be just as visible to the caller as writing +// into its map. The slice is built with room to spare so that an append within +// capacity cannot hide behind an unchanged length. +func TestJSONHandlerDoesNotMutateCallerSlice(t *testing.T) { + caller := make([]any, 2, 4) + caller[0] = "first" + caller[1] = "second" + + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord( + "casting", + slog.Any("s", caller), + slog.Group("s", slog.Int("injected", 1)), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantUnchanged(t, "slice", caller, []any{"first", "second"}) + wantUnchanged( + t, + "slice backing array", + caller[:cap(caller)], + []any{"first", "second", nil, nil}, + ) + + group := wantGroup(t, decodeLine(t, output), "s") + wantField(t, group, "injected", float64(1)) +} + +func TestConsoleHandlerDoesNotMutateCallerMap(t *testing.T) { + caller := map[string]any{"mine": "untouched"} + + output := captureStdout(t, func() { + handler := NewConsoleHandler() + record := testRecord( + "casting", + slog.Any("g", caller), + slog.Group("g", slog.Int("injected", 1)), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantUnchanged(t, "map", caller, map[string]any{"mine": "untouched"}) + wantContains(t, output, "g.injected=1") +} + +func TestWebhookHandlerDoesNotMutateCallerMap(t *testing.T) { + caller := map[string]any{"id": "req-1"} + + bodies := make(chan []byte, 1) + server := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read webhook body: %v", err) + } + bodies <- body + w.WriteHeader(http.StatusOK) + }, + )) + defer server.Close() + + handler, err := NewWebhookHandler(server.URL) + if err != nil { + t.Fatalf("NewWebhookHandler: %v", err) + } + + derived := handler. + WithAttrs([]slog.Attr{slog.Any("req", caller)}). + WithGroup("req") + record := testRecord("cast failed", slog.Int("status", 502)) + if err := derived.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + + var body []byte + select { + case body = <-bodies: + case <-time.After(5 * time.Second): + t.Fatal("webhook handler posted nothing") + } + + wantUnchanged(t, "map", caller, map[string]any{"id": "req-1"}) + + group := wantGroup(t, decodeLine(t, string(body)), "req") + wantField(t, group, "status", float64(502)) +} + +// TestJSONHandlerRendersDurationAsNanoseconds pins the json rendering of a +// duration to a number of nanoseconds, which is what slog.NewJSONHandler +// emits. A consumer that sums or compares the field needs a number; the "3s" +// form would silently arrive as a string. +func TestJSONHandlerRendersDurationAsNanoseconds(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord("casting", slog.Duration("elapsed", 3*time.Second)) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + if _, isNumber := decoded["elapsed"].(float64); !isNumber { + t.Fatalf("field \"elapsed\" = %#v, want a json number", decoded["elapsed"]) + } + wantField(t, decoded, "elapsed", float64((3 * time.Second).Nanoseconds())) +} + +// TestConsoleHandlerRendersDurationReadably is the other half of that choice: +// the console line is read by a person, so it carries the same human-readable +// form slog.NewTextHandler uses. +func TestConsoleHandlerRendersDurationReadably(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler() + record := testRecord("casting", slog.Duration("elapsed", 3*time.Second)) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, "elapsed=3s") +} + +// TestJSONHandlerContractEdgeCases pins the slog.Handler contract's four +// edge cases. Every case carries a control attribute as well, so a handler +// that emitted no attributes at all would fail rather than pass by omission. +// +// The empty group is attached through WithAttrs rather than to the record, +// because slog.Record.AddAttrs elides empty groups itself: routed through the +// record, the case would never reach the handler at all. +func TestJSONHandlerContractEdgeCases(t *testing.T) { + tests := []struct { + name string + build func() slog.Handler + attrs []slog.Attr + verify func(t *testing.T, decoded map[string]any) + }{ + { + name: "empty attr is ignored", + build: func() slog.Handler { return NewJSONHandler() }, + attrs: []slog.Attr{{}, slog.String("kept", "yes")}, + verify: func(t *testing.T, decoded map[string]any) { + wantField(t, decoded, "kept", "yes") + wantNoField(t, decoded, "") + }, + }, + { + name: "empty group is elided along with its key", + build: func() slog.Handler { + return NewJSONHandler(). + WithAttrs([]slog.Attr{slog.Group("empty")}) + }, + attrs: []slog.Attr{slog.String("kept", "yes")}, + verify: func(t *testing.T, decoded map[string]any) { + wantField(t, decoded, "kept", "yes") + wantNoField(t, decoded, "empty") + }, + }, + { + name: "group with an empty key is inlined", + build: func() slog.Handler { return NewJSONHandler() }, + attrs: []slog.Attr{ + slog.Group("", slog.String("inner", "yes")), + }, + verify: func(t *testing.T, decoded map[string]any) { + wantField(t, decoded, "inner", "yes") + wantNoField(t, decoded, "") + }, + }, + { + name: "WithGroup with an empty name is a no-op", + build: func() slog.Handler { + return NewJSONHandler().WithGroup("") + }, + attrs: []slog.Attr{slog.String("kept", "yes")}, + verify: func(t *testing.T, decoded map[string]any) { + wantField(t, decoded, "kept", "yes") + wantNoField(t, decoded, "") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + output := captureStdout(t, func() { + handler := test.build() + record := testRecord("casting", test.attrs...) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + test.verify(t, decodeLine(t, output)) + }) + } +} + +// TestConsoleHandlerContractEdgeCases holds the console handler to the same +// four rules, since it renders attributes through its own code path. +func TestConsoleHandlerContractEdgeCases(t *testing.T) { + tests := []struct { + name string + build func() slog.Handler + attrs []slog.Attr + want string + notWant string + }{ + { + name: "empty attr is ignored", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{{}, slog.String("kept", "yes")}, + want: "kept=yes", + notWant: " =", + }, + { + name: "empty group is elided along with its key", + build: func() slog.Handler { + return NewConsoleHandler(). + WithAttrs([]slog.Attr{slog.Group("empty")}) + }, + attrs: []slog.Attr{slog.String("kept", "yes")}, + want: "kept=yes", + notWant: "empty=", + }, + { + name: "group with an empty key is inlined", + build: func() slog.Handler { return NewConsoleHandler() }, + attrs: []slog.Attr{ + slog.Group("", slog.String("inner", "yes")), + }, + want: " inner=yes", + notWant: ".inner=", + }, + { + name: "WithGroup with an empty name is a no-op", + build: func() slog.Handler { + return NewConsoleHandler().WithGroup("") + }, + attrs: []slog.Attr{slog.String("kept", "yes")}, + want: " kept=yes", + notWant: ".kept=", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + output := captureStdout(t, func() { + handler := test.build() + record := testRecord("casting", test.attrs...) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, test.want) + wantNotContains(t, output, test.notWant) + }) + } +} + +// TestJSONHandlerRecordFieldsWinKeyCollision pins a caller-visible surprise: +// the json payload is an object, so the record's own fields own their names +// and an attribute keyed after one of them is dropped rather than emitted. +func TestJSONHandlerRecordFieldsWinKeyCollision(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord( + "the real message", + slog.String("Time", "hijacked"), + slog.String("Level", "hijacked"), + slog.String("Message", "hijacked"), + slog.String("PC", "hijacked"), + slog.String("kept", "yes"), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + decoded := decodeLine(t, output) + wantField(t, decoded, "kept", "yes") + wantField(t, decoded, "Message", "the real message") + wantField(t, decoded, "Level", "INFO") + for _, reserved := range []string{"Time", "PC"} { + if decoded[reserved] == "hijacked" { + t.Fatalf("attribute overwrote the record's own %q field: %v", reserved, decoded) + } + } +} + +// TestJSONHandlerDuplicateKeysKeepLast pins the other consequence of the +// payload being an object: a key logged twice collapses to its last value. +func TestJSONHandlerDuplicateKeysKeepLast(t *testing.T) { + output := captureStdout(t, func() { + handler := NewJSONHandler() + record := testRecord( + "casting", + slog.String("device", "kitchen"), + slog.String("device", "livingroom"), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantField(t, decodeLine(t, output), "device", "livingroom") +} + +// TestConsoleHandlerKeepsDuplicateKeys is the console counterpart: a line of +// text is not an object, so both pairs survive there. +func TestConsoleHandlerKeepsDuplicateKeys(t *testing.T) { + output := captureStdout(t, func() { + handler := NewConsoleHandler() + record := testRecord( + "casting", + slog.String("device", "kitchen"), + slog.String("device", "livingroom"), + ) + if err := handler.Handle(context.Background(), record); err != nil { + t.Fatalf("Handle: %v", err) + } + }) + + wantContains(t, output, "device=kitchen") + wantContains(t, output, "device=livingroom") +} -- 2.49.1 From 430bd762302eaaeebd73be2cfc2706f4f2bf4199 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 10 Aug 2026 12:59:58 +0000 Subject: [PATCH 2/2] Emit slog attributes from every handler (closes #19) The handlers took attributes and dropped them on the floor. Handle never read record.Attrs, so the inline slog.Info("casting", "device", d) form lost its fields; WithAttrs returned the receiver unchanged, so anything attached to a derived logger vanished; and WithGroup did the same, so grouping silently did nothing. The JSON and webhook handlers marshaled the slog.Record value directly, which cannot work: a record keeps its attributes in unexported fields, so encoding/json only ever saw Time, Message, Level and PC. The consequence was perverse. Converting log.Printf("[%s] Casting %s", device, file) into structured attributes, as the Go styleguide asks, left the output with strictly less information than before, and the calling code reviewed as correct because it was correct. A small shared attribute layer now holds the accumulated attributes and open groups. It is copy-on-write, so two loggers derived from one parent cannot leak attributes into each other, and it qualifies attributes by the groups open at the time they were attached, per the slog.Handler contract. Rendering follows each handler's format: the JSON and webhook handlers emit attributes as object fields with groups as nested objects, merging a group named twice rather than duplicating its key; the console handler appends key=value pairs, groups flattened to dotted keys, with key and value each quoted where leaving it bare would be ambiguous. The key is quoted as one token, prefix included, so the "=" that separates the pair is always the first one outside quotes: a key of "a=b" reads as "a=b"=v rather than as a=b=v, which parses as the key "a" holding the value "b=v". That is what slog.NewTextHandler does, and the console rendering was compared against it key by key. Values the caller logged go into the json payload by reference, because rendering only reads them, which leaves the group merge as the one place a value already in the payload is written to. It merges only into the unexported groupMap type this package allocates for its own groups: a caller's map[string]any is a different type and can never satisfy that type assertion, so it is replaced rather than written into. The invariant holds by construction - nothing reachable from the caller is modified by logging it. Values are resolved through slog.Value.Resolve, so LogValuer values are reported as the value they stand for instead of as a struct, and errors are reported as their message rather than as the empty object encoding/json makes of them. Anything encoding/json cannot marshal falls back to its slog string form rather than rendering as an empty object. A duration is nanoseconds as a number in the json and webhook payloads, matching slog.NewJSONHandler, so a consumer can compare and aggregate the field without parsing it first. The console line keeps the readable "3s" form, matching slog.NewTextHandler, because a person reads that one. The record's own fields keep the names they have always had - Time, Level, Message, PC - and win a collision with an attribute key, so existing consumers of the json output see no change beyond the added fields. That, and a repeated key keeping its last value - except where both are groups, which merge - are the two ways an attribute can go missing from the json output; both are now written down in the README, merge exception included, rather than left to be discovered. All three handlers are covered, including WebhookHandler, which had the same defect and is reached through the same MultiplexHandler. --- README.md | 44 ++++++++ TODO.md | 6 ++ attrs.go | 255 +++++++++++++++++++++++++++++++++++++++++++++ console_handler.go | 17 ++- json_handler.go | 19 +++- webhook_handler.go | 19 +++- 6 files changed, 349 insertions(+), 11 deletions(-) create mode 100644 attrs.go diff --git a/README.md b/README.md index 3f6222e..fcc7e09 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,13 @@ Released v1.0.0 2024-06-14. Works as intended. No known bugs. - if output is a tty, outputs pretty color logs - if output is not a tty, outputs json - supports delivering each log message via a webhook +- emits every `slog` attribute: those passed to a log call, those + accumulated with `WithAttrs`, and those qualified by `WithGroup`. + `slog.Group` values nest, and `slog.LogValuer` values are resolved. In + json output attributes are object fields (groups become nested objects); + in console output they are appended as `key=value` pairs, with grouped + keys written as `group.key=value`. See + [Attribute output](#attribute-output) for the details worth knowing ## Planned Features @@ -60,6 +67,43 @@ func main() { } ``` +## Attribute output + +Attributes reach every handler: the ones passed to the log call, the ones +accumulated with `WithAttrs`, and the ones qualified by the groups open at +the time they were attached. `slog.Group` values nest, and +`slog.LogValuer` values are resolved to the value they stand for. A few +behaviours are worth knowing before you rely on them. + +**Your values are never modified.** Whatever you log is read and rendered, +never written to. A map or a slice you pass to `slog.Any` comes back from +the logger exactly as you handed it over, even when a group later uses the +same key. + +**Durations are nanoseconds in json, and readable on the console.** The +json and webhook payloads emit a `slog.Duration` as a number of +nanoseconds, matching `slog.NewJSONHandler`, so a consumer can compare and +aggregate the field without parsing it. The console line emits the same +duration as `3s`, matching `slog.NewTextHandler`, because a person reads +it. + +**The json payload is an object, with the consequences an object has.** +The record's own fields are named `Time`, `Level`, `Message` and `PC`, and +they own those names: an attribute keyed after one of them is dropped from +the json and webhook output. A key logged more than once keeps its last +value there for the same reason, with one exception: when both are +`slog.Group` values sharing a key, the two groups are merged into one +object holding the members of both, rather than the second replacing the +first. Neither the collision nor the merge applies to the console output, +which is a line of text: every pair appears, in order. If you need a field +called `message`, pick a key that does not collide - the collision is +silent. + +**Empty things follow the `slog.Handler` contract.** An empty `Attr` is +ignored, an empty group is elided along with its key, a group with an +empty key is inlined into its parent, and `WithGroup("")` returns the +handler unchanged. + ## License [WTFPL](./LICENSE) diff --git a/TODO.md b/TODO.md index cbd9a8d..9a9aaec 100644 --- a/TODO.md +++ b/TODO.md @@ -24,6 +24,10 @@ files it depends on: .golangci.yml, REPO_POLICIES.md, .editorconfig, # Completed Steps +* 2026-08-10: fixed every handler discarding slog attributes: console, + JSON and webhook handlers now emit record attributes, accumulate + WithAttrs without mutating the receiver, and honour WithGroup; + slog.Group values nest and LogValuer values are resolved * 2026-02-08: fixed JSONHandler deadlock from recursive log.Println, with regression test; tagged 1.0.1 * 2024-06-14: 1.0 prep: lint and fmt enforced in Docker build, call @@ -46,6 +50,8 @@ files it depends on: .golangci.yml, REPO_POLICIES.md, .editorconfig, the working tree * Pick one tag scheme before the next release (v1.0.0 vs 1.0.1 are inconsistent) +* Tag v1.0.2, with the leading v, once the attribute fix lands, so + consuming repos can move off pseudo-version pins in one step * Fix RELP output to cache (from old TODO) * Re-add RELP delivery over TCP to remote rsyslog imrelp; removed 2024-06-14 because it did not build (README planned feature) diff --git a/attrs.go b/attrs.go new file mode 100644 index 0000000..0bc840f --- /dev/null +++ b/attrs.go @@ -0,0 +1,255 @@ +package simplelog + +import ( + "encoding/json" + "log/slog" + "strconv" + "strings" + "unicode" +) + +// handlerAttrs is the attribute state every handler carries: the attributes +// accumulated by WithAttrs, plus the groups opened by WithGroup. Its methods +// never mutate the receiver, so handlers derived from a common parent stay +// independent of each other. +type handlerAttrs struct { + attrs []slog.Attr + groups []string +} + +// withAttrs returns a copy carrying attrs in addition to those already held. +// The attributes are qualified by the groups that are open at the time they +// are attached, as the slog.Handler contract requires. +func (h handlerAttrs) withAttrs(attrs []slog.Attr) handlerAttrs { + qualified := qualifyAttrs(h.groups, attrs) + combined := make([]slog.Attr, 0, len(h.attrs)+len(qualified)) + combined = append(combined, h.attrs...) + combined = append(combined, qualified...) + return handlerAttrs{attrs: combined, groups: h.groups} +} + +// withGroup returns a copy with a further group open. An empty name is a no-op, +// per the slog.Handler contract. +func (h handlerAttrs) withGroup(name string) handlerAttrs { + if name == "" { + return h + } + groups := make([]string, 0, len(h.groups)+1) + groups = append(groups, h.groups...) + groups = append(groups, name) + return handlerAttrs{attrs: h.attrs, groups: groups} +} + +// forRecord returns the accumulated attributes followed by the record's own, +// the latter qualified by any open groups. +func (h handlerAttrs) forRecord(record slog.Record) []slog.Attr { + own := qualifyAttrs(h.groups, recordAttrs(record)) + all := make([]slog.Attr, 0, len(h.attrs)+len(own)) + all = append(all, h.attrs...) + all = append(all, own...) + return all +} + +// qualifyAttrs nests attrs inside the given open groups, innermost last. +func qualifyAttrs(groups []string, attrs []slog.Attr) []slog.Attr { + for i := len(groups) - 1; i >= 0; i-- { + attrs = []slog.Attr{{ + Key: groups[i], + Value: slog.GroupValue(attrs...), + }} + } + return attrs +} + +// recordAttrs collects the attributes a record carries. They live in +// unexported fields, so they are only reachable through Record.Attrs - which +// is why marshaling a slog.Record directly loses every one of them. +func recordAttrs(record slog.Record) []slog.Attr { + attrs := make([]slog.Attr, 0, record.NumAttrs()) + record.Attrs(func(attr slog.Attr) bool { + attrs = append(attrs, attr) + return true + }) + return attrs +} + +// groupMap is a JSON object this package built for itself: the payload of a +// record, or one of the nested objects a slog.Group becomes. +// +// The distinct type is what keeps the handler from writing into data the +// caller still owns. A value handed to slog.Any goes into the payload by +// reference - copying every logged map and slice would be a real cost for no +// gain, since rendering only reads. The one place the handler writes into a +// value already in the payload is the group merge below, and a type assertion +// to groupMap can only succeed on a map this package allocated: a caller's +// map[string]any is a different type and never matches, however it was keyed. +// So the invariant holds by construction - nothing reachable from the caller +// is ever written to, only read. +type groupMap map[string]any + +// recordToMap renders a record, with its handler's attributes, as the JSON +// object the JSON and webhook handlers emit. The record's own fields keep the +// names they have always had, and win a collision with an attribute key. +func recordToMap(record slog.Record, attrs handlerAttrs) groupMap { + fields := attrsToMap(attrs.forRecord(record)) + fields["Time"] = record.Time + fields["Level"] = record.Level + fields["Message"] = record.Message + fields["PC"] = record.PC + return fields +} + +// attrsToMap renders attributes as a JSON object in which groups are nested +// objects. A group named more than once is merged rather than duplicated; +// any other repeated key keeps the last value, as a JSON object must. +func attrsToMap(attrs []slog.Attr) groupMap { + fields := make(groupMap, len(attrs)) + for _, attr := range attrs { + addAttrToMap(fields, attr) + } + return fields +} + +func addAttrToMap(fields groupMap, attr slog.Attr) { + value := attr.Value.Resolve() + if attr.Key == "" && value.Any() == nil { + // An empty Attr is ignored, per the slog.Handler contract. + return + } + + if value.Kind() == slog.KindGroup { + group := value.Group() + if len(group) == 0 { + // An empty group is elided, as is its key. + return + } + // A group with an empty key is inlined into its parent. + target := fields + if attr.Key != "" { + // Merge only into a group this package built. Anything else + // at this key - including a map the caller logged - is + // replaced rather than written into, so the caller's own + // data structure is never touched. + nested, ok := fields[attr.Key].(groupMap) + if !ok { + nested = make(groupMap, len(group)) + fields[attr.Key] = nested + } + target = nested + } + for _, member := range group { + addAttrToMap(target, member) + } + return + } + + fields[attr.Key] = jsonValue(value) +} + +// jsonValue converts a resolved slog.Value into something encoding/json can +// render usefully. Values it cannot marshal - and errors, which marshal to an +// empty object - fall back to their slog string form, so a value is never +// rendered as an empty object. +// +// Values are returned as they were given, not copied: nothing here or in its +// callers writes to a value the caller supplied. +func jsonValue(value slog.Value) any { + switch value.Kind() { + case slog.KindString: + return value.String() + case slog.KindInt64: + return value.Int64() + case slog.KindUint64: + return value.Uint64() + case slog.KindFloat64: + return value.Float64() + case slog.KindBool: + return value.Bool() + case slog.KindDuration: + // Nanoseconds as a number, which is what slog.NewJSONHandler + // emits. A JSON consumer can then compare and aggregate the + // field; the "3s" form would have to be parsed first, and no + // common log pipeline knows how. + return value.Duration().Nanoseconds() + case slog.KindTime: + return value.Time() + default: + // KindAny, and anything a future Go release adds. + return jsonAnyValue(value) + } +} + +func jsonAnyValue(value slog.Value) any { + held := value.Any() + if _, ok := held.(json.Marshaler); !ok { + if err, ok := held.(error); ok { + return err.Error() + } + } + if _, err := json.Marshal(held); err != nil { + return value.String() + } + return held +} + +// attrsToText renders attributes as the space separated key=value pairs the +// console handler appends to a log line. Groups become dotted key prefixes. +// +// Values take their slog string form, so a duration reads as "3s" here where +// the json output carries nanoseconds as a number. That is the same split the +// stdlib makes between slog.NewTextHandler and slog.NewJSONHandler: the +// console line is read by a person, the json line by a program. +func attrsToText(attrs []slog.Attr) string { + var out strings.Builder + for _, attr := range attrs { + appendAttrText(&out, "", attr) + } + return out.String() +} + +func appendAttrText(out *strings.Builder, prefix string, attr slog.Attr) { + value := attr.Value.Resolve() + if attr.Key == "" && value.Any() == nil { + return + } + + if value.Kind() == slog.KindGroup { + group := value.Group() + if len(group) == 0 { + return + } + nested := prefix + if attr.Key != "" { + nested = prefix + attr.Key + "." + } + for _, member := range group { + appendAttrText(out, nested, member) + } + return + } + + out.WriteString(" ") + // The key is quoted on the same terms as the value, and as a whole + // including its group prefix, because that is the token a reader has to + // find the "=" in. slog.NewTextHandler quotes "prefix+key" the same way, + // so a key like "a=b" reads as "a=b"=v2 rather than the ambiguous + // a=b=v2, which parses as the key "a" with the value "b=v2". + out.WriteString(quoteIfNeeded(prefix + attr.Key)) + out.WriteString("=") + out.WriteString(quoteIfNeeded(value.String())) +} + +// quoteIfNeeded quotes a key or a value only when leaving it bare would make +// the key=value pairs ambiguous, matching how the stdlib text handler reads. +func quoteIfNeeded(text string) string { + if text == "" { + return `""` + } + for _, r := range text { + if unicode.IsSpace(r) || !unicode.IsPrint(r) || + r == '"' || r == '=' { + return strconv.Quote(text) + } + } + return text +} diff --git a/console_handler.go b/console_handler.go index 87552c9..5b60063 100644 --- a/console_handler.go +++ b/console_handler.go @@ -10,7 +10,9 @@ import ( "github.com/fatih/color" ) -type ConsoleHandler struct{} +type ConsoleHandler struct { + attrs handlerAttrs +} func NewConsoleHandler() *ConsoleHandler { return &ConsoleHandler{} @@ -42,12 +44,13 @@ func (c *ConsoleHandler) Handle( } fmt.Println( colorFunc( - "%s [%s] %s:%d: %s", + "%s [%s] %s:%d: %s%s", timestamp, record.Level, file, line, record.Message, + attrsToText(c.attrs.forRecord(record)), ), ) return nil @@ -61,9 +64,15 @@ func (c *ConsoleHandler) Enabled( } func (c *ConsoleHandler) WithAttrs(attrs []slog.Attr) slog.Handler { - return c + if len(attrs) == 0 { + return c + } + return &ConsoleHandler{attrs: c.attrs.withAttrs(attrs)} } func (c *ConsoleHandler) WithGroup(name string) slog.Handler { - return c + if name == "" { + return c + } + return &ConsoleHandler{attrs: c.attrs.withGroup(name)} } diff --git a/json_handler.go b/json_handler.go index 4c1c95b..3f77cb7 100644 --- a/json_handler.go +++ b/json_handler.go @@ -8,14 +8,19 @@ import ( "os" ) -type JSONHandler struct{} +type JSONHandler struct { + attrs handlerAttrs +} func NewJSONHandler() *JSONHandler { return &JSONHandler{} } func (j *JSONHandler) Handle(ctx context.Context, record slog.Record) error { - jsonData, _ := json.Marshal(record) + jsonData, err := json.Marshal(recordToMap(record, j.attrs)) + if err != nil { + return fmt.Errorf("error marshaling log record: %w", err) + } fmt.Fprintln(os.Stdout, string(jsonData)) return nil } @@ -25,9 +30,15 @@ func (j *JSONHandler) Enabled(ctx context.Context, level slog.Level) bool { } func (j *JSONHandler) WithAttrs(attrs []slog.Attr) slog.Handler { - return j + if len(attrs) == 0 { + return j + } + return &JSONHandler{attrs: j.attrs.withAttrs(attrs)} } func (j *JSONHandler) WithGroup(name string) slog.Handler { - return j + if name == "" { + return j + } + return &JSONHandler{attrs: j.attrs.withGroup(name)} } diff --git a/webhook_handler.go b/webhook_handler.go index 2c377aa..b2bbb7f 100644 --- a/webhook_handler.go +++ b/webhook_handler.go @@ -12,6 +12,7 @@ import ( type WebhookHandler struct { webhookURL string + attrs handlerAttrs } func (w *WebhookHandler) Enabled(ctx context.Context, level slog.Level) bool { @@ -19,11 +20,23 @@ func (w *WebhookHandler) Enabled(ctx context.Context, level slog.Level) bool { } func (w *WebhookHandler) WithAttrs(attrs []slog.Attr) slog.Handler { - return w + if len(attrs) == 0 { + return w + } + return &WebhookHandler{ + webhookURL: w.webhookURL, + attrs: w.attrs.withAttrs(attrs), + } } func (w *WebhookHandler) WithGroup(name string) slog.Handler { - return w + if name == "" { + return w + } + return &WebhookHandler{ + webhookURL: w.webhookURL, + attrs: w.attrs.withGroup(name), + } } func NewWebhookHandler(webhookURL string) (*WebhookHandler, error) { @@ -34,7 +47,7 @@ func NewWebhookHandler(webhookURL string) (*WebhookHandler, error) { } func (w *WebhookHandler) Handle(ctx context.Context, record slog.Record) error { - jsonData, err := json.Marshal(record) + jsonData, err := json.Marshal(recordToMap(record, w.attrs)) if err != nil { return fmt.Errorf("error marshaling event: %v", err) } -- 2.49.1