git.iko.soy/crush
on main help sign in
fix(posthog): check correct error; prevent panic (#2036)
216f904Christian Rocha
internal/event/event.go+5−5
compare with main
unifiedsplit
@@ -82,18 +82,18 @@ func send(event string, props ...any) {
82}
82}
83
83
84// Error logs an error event to PostHog with the error type and message.
84// Error logs an error event to PostHog with the error type and message.
85−func Error(err any, props ...any) {
85+func Error(errToLog any, props ...any) {
86 if client == nil {
86 if client == nil {
87 return
87 return
88 }
88 }
89 posthogErr := client.Enqueue(posthog.NewDefaultException(
89 posthogErr := client.Enqueue(posthog.NewDefaultException(
90 time.Now(),
90 time.Now(),
91 distinctId,
91 distinctId,
92− reflect.TypeOf(err).String(),
92+ reflect.TypeOf(errToLog).String(),
93− fmt.Sprintf("%v", err),
93+ fmt.Sprintf("%v", errToLog),
94 ))
94 ))
95− if err != nil {
95+ if posthogErr != nil {
96− slog.Error("Failed to enqueue PostHog error", "err", err, "props", props, "posthogErr", posthogErr)
96+ slog.Error("Failed to enqueue PostHog error", "err", errToLog, "props", props, "posthogErr", posthogErr)
97 return
97 return
98 }
98 }
99}
99}
internal/event/event_test.gonew file+74
@@ -0,0 +1,74 @@
1+package event
2+
3+// These tests verify that the Error function correctly handles various
4+// scenarios. These tests will not log anything.
5+
6+import (
7+ "testing"
8+)
9+
10+func TestError(t *testing.T) {
11+ t.Run("returns early when client is nil", func(t *testing.T) {
12+ // This test verifies that when the PostHog client is not initialized
13+ // the Error function safely returns early without attempting to
14+ // enqueue any events. This is important during initialization or when
15+ // metrics are disabled, as we don't want the error reporting mechanism
16+ // itself to cause panics.
17+ originalClient := client
18+ defer func() {
19+ client = originalClient
20+ }()
21+
22+ client = nil
23+ Error("test error", "key", "value")
24+ })
25+
26+ t.Run("handles nil client without panicking", func(t *testing.T) {
27+ // This test covers various edge cases where the error value might be
28+ // nil, a string, or an error type.
29+ originalClient := client
30+ defer func() {
31+ client = originalClient
32+ }()
33+
34+ client = nil
35+ Error(nil)
36+ Error("some error")
37+ Error(newDefaultTestError("runtime error"), "key", "value")
38+ })
39+
40+ t.Run("handles error with properties", func(t *testing.T) {
41+ // This test verifies that the Error function can handle additional
42+ // key-value properties that provide context about the error. These
43+ // properties are typically passed when recovering from panics (i.e.,
44+ // panic name, function name).
45+ //
46+ // Even with these additional properties, the function should handle
47+ // them gracefully without panicking.
48+ originalClient := client
49+ defer func() {
50+ client = originalClient
51+ }()
52+
53+ client = nil
54+ Error("test error",
55+ "type", "test",
56+ "severity", "high",
57+ "source", "unit-test",
58+ )
59+ })
60+}
61+
62+// newDefaultTestError creates a test error that mimics runtime panic
63+// errors. This helps us testing that the Error function can handle various
64+// error types, including those that might be passed from a panic recovery
65+// scenario.
66+func newDefaultTestError(s string) error {
67+ return testError(s)
68+}
69+
70+type testError string
71+
72+func (e testError) Error() string {
73+ return string(e)
74+}