Convenience #2
Convenience #2
Conversation
|
Great, this is the right direction. I'll add some inline comments. |
| packet.Tags = args | ||
| client.Report(packet) | ||
|
|
||
| return packet.EventID |
There was a problem hiding this comment.
Report is async, so this won't work as expected. Also it looks like we aren't parsing the ID from the response (we should for the synchronous Send).
There was a problem hiding this comment.
Here's an idea.
Currently, the Sentry docs encourage you to do async submission whenever possible:
Additionally, the following features are highly encouraged:
- Automated error handling (e.g. default error handlers)
- Logging integration (to whatever standard solution is available)
- Non-blocking event submission
- Basic data sanitization (e.g. filtering out values that look like passwords)
This is clearly the desirable configuration in 99% of production cases, but it means that simple "hey look I can send" demos don't work if you don't make your goroutine sleep.
The raven-python client only has a send method, and automatically decides whether or not to send asynchronously based on the transport. We can decide whether or not to do that in a separate issue, but I think you're right that I should change this to Send for now.
|
Hey @titanous, here are some of the fixes we came up with. I want to try and make stacktraces (and what they include or don't include) smarter in the future, but that's another issue. |
|
Oh and Merry Christmas! 🎅 |
| defer func() { | ||
| var packet *Packet | ||
| rval := recover() | ||
| switch rval := rval.(type) { |
There was a problem hiding this comment.
switch rval := recover().(type) {
| @@ -54,6 +54,11 @@ type Transport interface { | |||
| Send(url, authHeader string, packet *Packet) error | |||
| } | |||
|
|
|||
| type outgoingPacket struct { | |||
| packet *Packet | |||
| cherr chan error | |||
There was a problem hiding this comment.
I'd rename this to ch, could also call it errChan.
|
I'm happy with this (pending my two minor notes). Is there anything else you want to do? When you're ready please rebase this into one or more atomic commits and I'll merge. |
|
I think I'm borking the rebase thing. I assume you want me to follow the guide here (invoking the wrath of the git gods, I am told) and do everything as a squash so it stops looking like 23 commits (made 49 by my borked rebase)? 👶 Thanks |
|
Yeah, it looks like you didn't do a force push and instead merged origin/convenience on top, causing the weirdness. You should be able to |
Fix supplying tags go fmt Recover dumbly from panics. Improve stack -> culprit display and string messages. Quick implementation of CapturePanic s/args/tags/ w/ some go fmt thrown in for good measure Note where a stacktrace is added. Handle case where there is no panic Just "Send", since this is async by default w/ golang transports. Condense recover-switch-type Very rough optionally-synchronous capture Fix drop handler Check for packet initialization errors Better tag merging Lil' bit cleaner code de-cherr-yfy Condense writing to error channel Buffer error channel Don’t want to slow down the worker. Good catch @titanous. Give a uniform error for dropped packets Fixes to Writer for a clean merge Hint map lengths de-cherr-yfy the example
|
Success! Yep, I was missing |
|
Awesome, thanks! |
|
No, thank you! |
fixed import paths in README.md
This is just me starting the convenience methods I pointed out in #1. Let me know if this is the direction you'd like this to go in!