Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error events can silently be lost #613

Open
svix-aaron1011 opened this issue Sep 13, 2023 · 2 comments
Open

Error events can silently be lost #613

svix-aaron1011 opened this issue Sep 13, 2023 · 2 comments

Comments

@svix-aaron1011
Copy link

Environment

sentry v0.31.5

Steps to Reproduce

There are multiple ways that Sentry can silently lose error events:

  • When any of the sentry transport backends fail to send a message, they only log this at debug level:
    sentry_debug!("Failed to send envelope: {}", err);

    If the off-by-default feature debug-logs is not enabled, then this won't get logged at all unless Sentry is configured with ClientOptions.debug = true (also off-by-default).
  • The TransportThread used for sending messages uses a channel with a size of 30. If this size is exceeded, then events will be dropped (with the same debug-logging issue as above):
    // Using send here would mean that when the channel fills up for whatever
    // reason, trying to send an envelope would block everything. We'd rather
    // drop the envelope in that case.
    if let Err(e) = self.sender.try_send(Task::SendEnvelope(envelope)) {
    sentry_debug!("envelope dropped: {e}");
    }

Expected Result

When using Sentry in production, it can be critical that any errors are either reported to sentry, or trigger some kind of error/panic indicating that an event was not reported.

It would be very useful for Sentry to:

  • Log these kinds of messages as errors, rather than off-by-default debug messages
  • Provide the ability to use an unbounded channel (or use an unbounded channel for error/exception events), to prevent events from getting dropped in the first place
@loewenheim
Copy link
Contributor

Thank you for the report, we'll discuss it.

@Swatinem
Copy link
Member

There is also the possibility of this being dropped in the transport due to rate limits and some such.

Ideally, all the capturing methods would a Future that resolves to a descriptive enum with various variants for the different reasons a message/exception might be lost.
However, that would be quite deep change to how things are working right now, and would require lots of effort to make happen.

I don’t think the idea of an unbounded channel, or different channels for different event types makes sense. The transports are completely agnostic to that, as they are only dealing with opaque envelopes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants