search

getsentry / raven-go

Sentry client in Go
https://sentry.io
BSD 3-Clause "New" or "Revised" License
561 stars 148 forks source link

With extra #190

Closed JackWink closed 6 years ago

JackWink commented 6 years ago

Hey there, I couldn't find a way to cleanly contribute this back, the original pr (#168) seems to have fallen off the radar. I rebased this PR off the latest master, added tests and a couple comments to the original code.

The first commit I added fixes tests for go 1.10 (and ran gofmt over client_test.go unfortunately). The actual relevant changes to look at are on lines 121 and 242. In short, fmt was failing to compile complaining that the string types (level) weren't integers (being formatted with %d) and then again on converting raven.Timestamp (time.Time) to a string. There may be other/better ways to solve these issues that I'd be happy to address, I just wanted to get the tests in working order.

The second commit adds the test cases and comments. One thing I noticed was that errors that are double wrapped retain the earliest set value ie. there is no value overriding when wrapping again. I opted to note the behavior and not change it, but it should be easy to use the latest defined value if that's the desired behavior.

JackWink commented 6 years ago

Just noticed this fixes #178 as well

JackWink commented 6 years ago

@mattrobenolt I rebased this on top of the gofmt commit so this PR should be cleaner/easier to grok, let me know if there's anything I can do to bump this up in priority. Thanks!

mattrobenolt commented 6 years ago

This is great! Thanks for the thorough tests. 🍕