From 5c975489238495f7e123d78ed3121de48e0e2491 Mon Sep 17 00:00:00 2001 From: Joshua Wood Date: Fri, 1 Aug 2025 19:44:42 -0700 Subject: [PATCH 1/5] fix: allow string and int types for backtrace number field --- internal/hbapi/types.go | 30 +++++++- internal/hbapi/types_test.go | 145 +++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 internal/hbapi/types_test.go diff --git a/internal/hbapi/types.go b/internal/hbapi/types.go index c8f9622..d71b76b 100644 --- a/internal/hbapi/types.go +++ b/internal/hbapi/types.go @@ -1,6 +1,32 @@ package hbapi -import "time" +import ( + "encoding/json" + "fmt" + "time" +) + +// StringOrInt is a custom type that can unmarshal from either a string or integer JSON value +type StringOrInt string + +// UnmarshalJSON implements json.Unmarshaler interface to handle both string and integer values +func (s *StringOrInt) UnmarshalJSON(data []byte) error { + // Try to unmarshal as string first + var str string + if err := json.Unmarshal(data, &str); err == nil { + *s = StringOrInt(str) + return nil + } + + // If that fails, try as integer + var num int + if err := json.Unmarshal(data, &num); err == nil { + *s = StringOrInt(fmt.Sprintf("%d", num)) + return nil + } + + return fmt.Errorf("StringOrInt: cannot unmarshal %s into string or integer", data) +} // User represents a Honeybadger user type User struct { @@ -95,7 +121,7 @@ type NoticeRequest struct { // BacktraceEntry represents a single entry in the error backtrace type BacktraceEntry struct { - Number string `json:"number"` + Number StringOrInt `json:"number"` File string `json:"file"` Method string `json:"method"` Source map[string]interface{} `json:"source,omitempty"` diff --git a/internal/hbapi/types_test.go b/internal/hbapi/types_test.go new file mode 100644 index 0000000..ebd18fc --- /dev/null +++ b/internal/hbapi/types_test.go @@ -0,0 +1,145 @@ +package hbapi + +import ( + "encoding/json" + "testing" +) + +func TestStringOrInt_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr bool + }{ + { + name: "string value", + input: `"123"`, + want: "123", + wantErr: false, + }, + { + name: "integer value", + input: `123`, + want: "123", + wantErr: false, + }, + { + name: "zero integer", + input: `0`, + want: "0", + wantErr: false, + }, + { + name: "negative integer", + input: `-1`, + want: "-1", + wantErr: false, + }, + { + name: "boolean value should fail", + input: `true`, + wantErr: true, + }, + { + name: "object value should fail", + input: `{}`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var s StringOrInt + err := json.Unmarshal([]byte(tt.input), &s) + if (err != nil) != tt.wantErr { + t.Errorf("StringOrInt.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr && string(s) != tt.want { + t.Errorf("StringOrInt.UnmarshalJSON() = %v, want %v", s, tt.want) + } + }) + } +} + +func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want BacktraceEntry + wantErr bool + }{ + { + name: "number as string", + input: `{ + "number": "42", + "file": "/path/to/file.js", + "method": "someMethod" + }`, + want: BacktraceEntry{ + Number: "42", + File: "/path/to/file.js", + Method: "someMethod", + }, + wantErr: false, + }, + { + name: "number as integer", + input: `{ + "number": 42, + "file": "/path/to/file.js", + "method": "someMethod" + }`, + want: BacktraceEntry{ + Number: "42", + File: "/path/to/file.js", + Method: "someMethod", + }, + wantErr: false, + }, + { + name: "complete backtrace entry with source", + input: `{ + "number": 10, + "file": "/app/index.js", + "method": "handleError", + "source": {"10": "throw new Error();"}, + "context": "app" + }`, + want: BacktraceEntry{ + Number: "10", + File: "/app/index.js", + Method: "handleError", + Source: map[string]interface{}{"10": "throw new Error();"}, + Context: "app", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var entry BacktraceEntry + err := json.Unmarshal([]byte(tt.input), &entry) + if (err != nil) != tt.wantErr { + t.Errorf("BacktraceEntry.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr { + if string(entry.Number) != string(tt.want.Number) { + t.Errorf("BacktraceEntry.Number = %v, want %v", entry.Number, tt.want.Number) + } + if entry.File != tt.want.File { + t.Errorf("BacktraceEntry.File = %v, want %v", entry.File, tt.want.File) + } + if entry.Method != tt.want.Method { + t.Errorf("BacktraceEntry.Method = %v, want %v", entry.Method, tt.want.Method) + } + if entry.Context != tt.want.Context { + t.Errorf("BacktraceEntry.Context = %v, want %v", entry.Context, tt.want.Context) + } + } + }) + } +} From e91670d0a53140e9e5a2d28bc4761d16b9d6b926 Mon Sep 17 00:00:00 2001 From: Joshua Wood Date: Fri, 1 Aug 2025 19:54:54 -0700 Subject: [PATCH 2/5] fix: add missing optional backtrace field types --- internal/hbapi/types.go | 4 +++ internal/hbapi/types_test.go | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/internal/hbapi/types.go b/internal/hbapi/types.go index d71b76b..9218c53 100644 --- a/internal/hbapi/types.go +++ b/internal/hbapi/types.go @@ -122,8 +122,12 @@ type NoticeRequest struct { // BacktraceEntry represents a single entry in the error backtrace type BacktraceEntry struct { Number StringOrInt `json:"number"` + Column *StringOrInt `json:"column,omitempty"` File string `json:"file"` Method string `json:"method"` + Class string `json:"class,omitempty"` + Type string `json:"type,omitempty"` + Args []interface{} `json:"args,omitempty"` Source map[string]interface{} `json:"source,omitempty"` Context string `json:"context,omitempty"` } diff --git a/internal/hbapi/types_test.go b/internal/hbapi/types_test.go index ebd18fc..6e5c330 100644 --- a/internal/hbapi/types_test.go +++ b/internal/hbapi/types_test.go @@ -116,6 +116,48 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { }, wantErr: false, }, + { + name: "backtrace entry with all optional fields", + input: `{ + "number": "25", + "column": 15, + "file": "/app/models/user.rb", + "method": "authenticate", + "class": "User", + "type": "instance", + "args": ["email@example.com", "password"], + "source": {"25": "user = User.find_by(email: email)"}, + "context": "app" + }`, + want: BacktraceEntry{ + Number: "25", + Column: func() *StringOrInt { s := StringOrInt("15"); return &s }(), + File: "/app/models/user.rb", + Method: "authenticate", + Class: "User", + Type: "instance", + Args: []interface{}{"email@example.com", "password"}, + Source: map[string]interface{}{"25": "user = User.find_by(email: email)"}, + Context: "app", + }, + wantErr: false, + }, + { + name: "backtrace entry with column as string", + input: `{ + "number": 42, + "column": "8", + "file": "/lib/helper.js", + "method": "processData" + }`, + want: BacktraceEntry{ + Number: "42", + Column: func() *StringOrInt { s := StringOrInt("8"); return &s }(), + File: "/lib/helper.js", + Method: "processData", + }, + wantErr: false, + }, } for _, tt := range tests { @@ -139,6 +181,26 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { if entry.Context != tt.want.Context { t.Errorf("BacktraceEntry.Context = %v, want %v", entry.Context, tt.want.Context) } + // Check Column + if tt.want.Column != nil && entry.Column != nil { + if string(*entry.Column) != string(*tt.want.Column) { + t.Errorf("BacktraceEntry.Column = %v, want %v", *entry.Column, *tt.want.Column) + } + } else if (tt.want.Column == nil) != (entry.Column == nil) { + t.Errorf("BacktraceEntry.Column = %v, want %v", entry.Column, tt.want.Column) + } + // Check Class + if entry.Class != tt.want.Class { + t.Errorf("BacktraceEntry.Class = %v, want %v", entry.Class, tt.want.Class) + } + // Check Type + if entry.Type != tt.want.Type { + t.Errorf("BacktraceEntry.Type = %v, want %v", entry.Type, tt.want.Type) + } + // Check Args length + if len(entry.Args) != len(tt.want.Args) { + t.Errorf("BacktraceEntry.Args length = %v, want %v", len(entry.Args), len(tt.want.Args)) + } } }) } From 32bc9a0108e9b731cf378a69346ae58f84e7eca0 Mon Sep 17 00:00:00 2001 From: Joshua Wood Date: Fri, 1 Aug 2025 20:33:51 -0700 Subject: [PATCH 3/5] fix: coerce number/column fields to int instead of string --- internal/hbapi/types.go | 35 ++++++++----- internal/hbapi/types_test.go | 99 ++++++++++++++++++++++++++++-------- 2 files changed, 100 insertions(+), 34 deletions(-) diff --git a/internal/hbapi/types.go b/internal/hbapi/types.go index 9218c53..11bf0c8 100644 --- a/internal/hbapi/types.go +++ b/internal/hbapi/types.go @@ -3,29 +3,36 @@ package hbapi import ( "encoding/json" "fmt" + "strconv" "time" ) -// StringOrInt is a custom type that can unmarshal from either a string or integer JSON value -type StringOrInt string +// Number is a custom type that can unmarshal from either a string or integer JSON value +// and stores it as an integer +type Number int // UnmarshalJSON implements json.Unmarshaler interface to handle both string and integer values -func (s *StringOrInt) UnmarshalJSON(data []byte) error { - // Try to unmarshal as string first - var str string - if err := json.Unmarshal(data, &str); err == nil { - *s = StringOrInt(str) +func (n *Number) UnmarshalJSON(data []byte) error { + // Try to unmarshal as integer first + var num int + if err := json.Unmarshal(data, &num); err == nil { + *n = Number(num) return nil } - // If that fails, try as integer - var num int - if err := json.Unmarshal(data, &num); err == nil { - *s = StringOrInt(fmt.Sprintf("%d", num)) + // If that fails, try as string and parse to int + var str string + if err := json.Unmarshal(data, &str); err == nil { + // Try to parse the string as an integer + parsed, err := strconv.Atoi(str) + if err != nil { + return fmt.Errorf("Number: cannot parse string %q as integer: %v", str, err) + } + *n = Number(parsed) return nil } - return fmt.Errorf("StringOrInt: cannot unmarshal %s into string or integer", data) + return fmt.Errorf("Number: cannot unmarshal %s into integer or string", data) } // User represents a Honeybadger user @@ -121,8 +128,8 @@ type NoticeRequest struct { // BacktraceEntry represents a single entry in the error backtrace type BacktraceEntry struct { - Number StringOrInt `json:"number"` - Column *StringOrInt `json:"column,omitempty"` + Number Number `json:"number"` + Column *Number `json:"column,omitempty"` File string `json:"file"` Method string `json:"method"` Class string `json:"class,omitempty"` diff --git a/internal/hbapi/types_test.go b/internal/hbapi/types_test.go index 6e5c330..b603bf5 100644 --- a/internal/hbapi/types_test.go +++ b/internal/hbapi/types_test.go @@ -5,37 +5,48 @@ import ( "testing" ) -func TestStringOrInt_UnmarshalJSON(t *testing.T) { +func TestNumber_UnmarshalJSON(t *testing.T) { tests := []struct { name string input string - want string + want int wantErr bool }{ { name: "string value", input: `"123"`, - want: "123", + want: 123, wantErr: false, }, { name: "integer value", input: `123`, - want: "123", + want: 123, wantErr: false, }, { name: "zero integer", input: `0`, - want: "0", + want: 0, wantErr: false, }, { name: "negative integer", input: `-1`, - want: "-1", + want: -1, wantErr: false, }, + { + name: "string negative integer", + input: `"-42"`, + want: -42, + wantErr: false, + }, + { + name: "non-numeric string should fail", + input: `"abc"`, + wantErr: true, + }, { name: "boolean value should fail", input: `true`, @@ -50,14 +61,14 @@ func TestStringOrInt_UnmarshalJSON(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var s StringOrInt - err := json.Unmarshal([]byte(tt.input), &s) + var n Number + err := json.Unmarshal([]byte(tt.input), &n) if (err != nil) != tt.wantErr { - t.Errorf("StringOrInt.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("Number.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) return } - if !tt.wantErr && string(s) != tt.want { - t.Errorf("StringOrInt.UnmarshalJSON() = %v, want %v", s, tt.want) + if !tt.wantErr && int(n) != tt.want { + t.Errorf("Number.UnmarshalJSON() = %v, want %v", n, tt.want) } }) } @@ -78,7 +89,7 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { "method": "someMethod" }`, want: BacktraceEntry{ - Number: "42", + Number: 42, File: "/path/to/file.js", Method: "someMethod", }, @@ -92,7 +103,7 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { "method": "someMethod" }`, want: BacktraceEntry{ - Number: "42", + Number: 42, File: "/path/to/file.js", Method: "someMethod", }, @@ -108,7 +119,7 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { "context": "app" }`, want: BacktraceEntry{ - Number: "10", + Number: 10, File: "/app/index.js", Method: "handleError", Source: map[string]interface{}{"10": "throw new Error();"}, @@ -130,8 +141,8 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { "context": "app" }`, want: BacktraceEntry{ - Number: "25", - Column: func() *StringOrInt { s := StringOrInt("15"); return &s }(), + Number: 25, + Column: numberPtr(15), File: "/app/models/user.rb", Method: "authenticate", Class: "User", @@ -151,13 +162,55 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { "method": "processData" }`, want: BacktraceEntry{ - Number: "42", - Column: func() *StringOrInt { s := StringOrInt("8"); return &s }(), + Number: 42, + Column: numberPtr(8), File: "/lib/helper.js", Method: "processData", }, wantErr: false, }, + { + name: "number as string negative", + input: `{ + "number": "-10", + "file": "/app/test.rb", + "method": "test" + }`, + want: BacktraceEntry{ + Number: -10, + File: "/app/test.rb", + Method: "test", + }, + wantErr: false, + }, + { + name: "invalid string number", + input: `{ + "number": "abc", + "file": "/app/test.rb", + "method": "test" + }`, + wantErr: true, + }, + { + name: "number as boolean should fail", + input: `{ + "number": true, + "file": "/app/test.rb", + "method": "test" + }`, + wantErr: true, + }, + { + name: "column as boolean should fail", + input: `{ + "number": 1, + "column": true, + "file": "/app/test.rb", + "method": "test" + }`, + wantErr: true, + }, } for _, tt := range tests { @@ -169,7 +222,7 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { return } if !tt.wantErr { - if string(entry.Number) != string(tt.want.Number) { + if int(entry.Number) != int(tt.want.Number) { t.Errorf("BacktraceEntry.Number = %v, want %v", entry.Number, tt.want.Number) } if entry.File != tt.want.File { @@ -183,7 +236,7 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { } // Check Column if tt.want.Column != nil && entry.Column != nil { - if string(*entry.Column) != string(*tt.want.Column) { + if int(*entry.Column) != int(*tt.want.Column) { t.Errorf("BacktraceEntry.Column = %v, want %v", *entry.Column, *tt.want.Column) } } else if (tt.want.Column == nil) != (entry.Column == nil) { @@ -205,3 +258,9 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { }) } } + +// Helper function to create Number pointers +func numberPtr(i int) *Number { + n := Number(i) + return &n +} \ No newline at end of file From f01b5d5a9c3a721e0a823f559f7901860cea0a16 Mon Sep 17 00:00:00 2001 From: Joshua Wood Date: Fri, 1 Aug 2025 20:34:55 -0700 Subject: [PATCH 4/5] chore: format --- internal/hbapi/types_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/hbapi/types_test.go b/internal/hbapi/types_test.go index b603bf5..963748b 100644 --- a/internal/hbapi/types_test.go +++ b/internal/hbapi/types_test.go @@ -263,4 +263,4 @@ func TestBacktraceEntry_UnmarshalJSON(t *testing.T) { func numberPtr(i int) *Number { n := Number(i) return &n -} \ No newline at end of file +} From e9bb1bb8ca3f12ed3942540583178a050c3d072f Mon Sep 17 00:00:00 2001 From: Joshua Wood Date: Fri, 1 Aug 2025 20:41:21 -0700 Subject: [PATCH 5/5] chore: more generic error message Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/hbapi/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/hbapi/types.go b/internal/hbapi/types.go index 11bf0c8..fd19c64 100644 --- a/internal/hbapi/types.go +++ b/internal/hbapi/types.go @@ -32,7 +32,7 @@ func (n *Number) UnmarshalJSON(data []byte) error { return nil } - return fmt.Errorf("Number: cannot unmarshal %s into integer or string", data) + return fmt.Errorf("Number: cannot unmarshal value into integer or string") } // User represents a Honeybadger user