getsentry / raven-go

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

CapturePanic not sending packets #84

Open linusthe3rd opened 8 years ago

linusthe3rd commented 8 years ago

I've hit a weird issue where if I use raven.CapturePanic, no data is sent to the sentry server if log.Panic is called. I am not sure why this happens, but it seems like a bug.

Environment:

Example Code:

import (
    "log"
    "github.com/getsentry/raven-go"
)

func main() {
    raven.CapturePanic(func() {
        log.Panic("My Panic")
    }, nil)
}

When running this code, no exception occurs in my Sentry server. Digging into the raven codebase on my local machine, I added some print statements in client.go:Capture:

func (client *Client) Capture(packet *Packet, captureTags map[string]string) (eventID string, ch chan error) {
    if client == nil {
        log.Print("NIL CLIENT")
        return
    }

    // Keep track of all running Captures so that we can wait for them all to finish
    // *Must* call client.wg.Done() on any path that indicates that an event was
    // finished being acted upon, whether success or failure
    client.wg.Add(1)

    ch = make(chan error, 1)

    // Merge capture tags and client tags
    packet.AddTags(captureTags)
    packet.AddTags(client.Tags)
    packet.AddTags(client.context.tags)

    // Initialize any required packet fields
    client.mu.RLock()
    projectID := client.projectID
    release := client.release
    client.mu.RUnlock()

    err := packet.Init(projectID)
    if err != nil {
        ch <- err
        client.wg.Done()
        return
    }
    packet.Release = release

    outgoingPacket := &outgoingPacket{packet, ch}

    select {
    case client.queue <- outgoingPacket:
    default:
        log.Print("PACKET DROP")
        // Send would block, drop the packet
        if client.DropHandler != nil {
            client.DropHandler(packet)
        }
        ch <- ErrPacketDropped
        client.wg.Done()
    }

    log.Print("DUN CAPTURE")
    return packet.EventID, ch
}

I also added a print statement in `client.go:worker()':

func (client *Client) worker() {
    for outgoingPacket := range client.queue {

        log.Print("outgoing packet")
        client.mu.RLock()
        url, authHeader := client.url, client.authHeader
        client.mu.RUnlock()

        outgoingPacket.ch <- client.Transport.Send(url, authHeader, outgoingPacket.packet)
        client.wg.Done()
    }
}

Running the code with this setup, I get the following output:

2016/06/07 10:32:58 My Panic
2016/06/07 10:32:58 DUN CAPTURE

As you notice, there is no print statement for outgoing packet. This seems to indicate that the error message packet is not being added to the channel/client.queue and preventing the packet from being sent to the server before the program exits.

mattrobenolt commented 8 years ago

Is this related to #85 by any chance?

linusthe3rd commented 8 years ago

No, this is a separate issue.

On Jun 7, 2016, at 1:28 PM, Matt Robenolt notifications@github.com wrote:

Is this related to #85 by any chance?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

mattrobenolt commented 8 years ago

Alright, I'll investigate this today and see what's going on.

LeonLiuY commented 7 years ago

Any updates on this? After more than 1 year this still happens. Is this project dead?

mattrobenolt commented 7 years ago

It's not dead, we just have nobody in house to work on it. If someone would like to submit a PR, I'd be happy to review.

mattrobenolt commented 7 years ago

My only guess here is CapturePanic should be happening async and not waiting for your program to exit. There are other API methods which do block. Try using CapturePanicAndWait instead.

LeonLiuY commented 7 years ago

Nice to get response. Thanks!

I debugged into the code, and found the reason of my case was SSL certificate problem. I used self-signed certificate and trusted it in OS level, but this library is using github.com/certifi/gocertifi which is not using OS certificates.

I forked this and removed github.com/certifi/gocertifi to solve my issue. If you are OK with it I can send PR. https://github.com/liuyang1204/raven-go/commit/29da4ac72a0aa52ff6540eef5e38ad0d4be6000d

The essential problem is that the HTTP error is not informed anywhere so that I can only debug into the code. It should be logged at least to STDERR.

mattrobenolt commented 7 years ago

Ah, so that makes sense.

I'd accept a PR that made supplying certificates or some options around here a configurable option. The reason certifi was added in was because users were deploying their applications into containers without any system certificates, so figured it was pretty easy to just bundle them in with it.

LeonLiuY commented 7 years ago

Found the line responsible for the missing error message problem:

https://github.com/getsentry/raven-go/blob/master/client.go#L702

The error channel is ignored by this line. And same in other methods like CapturePanicAndWait

valentin-krasontovitsch commented 7 years ago

I would like to submit a PR that outputs something when a packet cannot be delivered (it seems like I'm having exactly the same problems as @linusthe3rd), and just for future reference, a link to the actual commit with the responsible line - the problem is in the client's Capture method.

Docs say:

A channel is provided if it is important to check for a send's success

That's the error channel @liuyang1204 was talking about (yeahy) - so we just have to use it...

Gonna investigate further...