getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
798 stars 315 forks source link

Pending reports are deleted when SDK is initialized with enabled = NO #572

Closed mirkoevo closed 2 years ago

mirkoevo commented 4 years ago

Platform:

Swift:

sentry-cocoa installed with:

Version of sentry-cocoa: 5.x.x


I have the following issue:

I have the need manually check the pending reports at startup, open a requester to let the user add a description and choose if send or discard the crash report. Unfortunately, if I disable the SDK at startup, all the pending reports are deleted.

Description goes here ...

Steps to reproduce:

[SentrySDK startWithOptions: @{ @"dsn": Sentry_dns,
                                @"enabled": @NO }];

if ([SentryCrash.sharedInstance reportCount] > 0) {

    [SentryCrash.sharedInstance sendAllReportsWithCompletion:^(NSArray *filteredReports, BOOL completed, NSError *error) {
     }
}

Actual result:

Pending reports are deleted in a sub call of -startWithOptions:

Expected result:

The method -[installation sendAllReports] should not be called in -[SentryCrashIntegration startCrashHandler] without checking options.enabled, but in the current implementation the SentryCrashReporterFilter is installed in SentryCrash only in -[SentryCrashInstallation sendAllReportsWithCompletion:]

philipphofmann commented 4 years ago

Thanks, @mirkoevo for reporting this issue. I found the cause(s) of this problem. In SentryCrashReportSink we convert the crash reports of SentryCrash to events and call [SentrySDK captureEvent:event]. We don't check there if the SDK is disabled and call onCompletion(sentReports, TRUE, nil), which is a callback to SentryCrash.sendAllReportsWithCompletion. So even if the SDK is disabled we tell the callback that we sent all reports successfully, which causes that we call sentrycrash_deleteAllReports. Furthermore we set self.deleteBehaviorAfterSendAll = SentryCrashCDeleteAlways; in SentryCrash.initWithBasePath, which leads to sentrycrash_deleteAllReports being called no matter if we could send the reports successfully or not.

In order to fix this, we need to change deleteBehaviorAfterSendAll in SentryCrash and we need to change that we always call onCompletion(sentReports, TRUE, nil); in SentryCrashReportSink. I think this is not going to be an easy fix. @HazAT do you maybe have any input on this?

philipphofmann commented 4 years ago

What could work for you @mirkoevo is to not call SentrySDK startWithOptions at all before the user decides on whether he wants to send the report or not. Once you know you can start the SDK and use beforeSend of SentryOptions to attach your description to the events.

github-actions[bot] commented 2 years ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

philipphofmann commented 2 years ago

As we didn't hear anything for a while, we are going to close this issue. Feel free to open it again.