- 
                Notifications
    You must be signed in to change notification settings 
- Fork 208
Added DeliveryStrategy to event delivery #2312
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
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package com.bugsnag.android | ||
|  | ||
| enum class DeliveryStrategy { | ||
| STORE_ONLY, | ||
| STORE_AND_FLUSH, | ||
| STORE_AND_SEND, | ||
| SEND_IMMEDIATELY, | ||
| } | 
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||
| package com.bugsnag.android; | ||||||
|  | ||||||
| import static com.bugsnag.android.SeverityReason.REASON_PROMISE_REJECTION; | ||||||
|  | ||||||
| import com.bugsnag.android.internal.ImmutableConfig; | ||||||
| import com.bugsnag.android.internal.InternalMetrics; | ||||||
|  | ||||||
|  | @@ -575,4 +577,47 @@ void setRedactedKeys(Collection<Pattern> redactedKeys) { | |||||
| void setInternalMetrics(InternalMetrics metrics) { | ||||||
| impl.setInternalMetrics(metrics); | ||||||
| } | ||||||
|  | ||||||
| /** | ||||||
| * Returns the delivery strategy for this event, which determines how the event | ||||||
| * should be delivered to the Bugsnag API. | ||||||
| * | ||||||
| * @return the delivery strategy, or null if no specific strategy is set | ||||||
| */ | ||||||
| @NonNull | ||||||
| public DeliveryStrategy getDeliveryStrategy() { | ||||||
| if (impl.getDeliveryStrategy() != null) { | ||||||
| return impl.getDeliveryStrategy(); | ||||||
| } | ||||||
|  | ||||||
| if (getImpl().getOriginalUnhandled()) { | ||||||
| String severityReasonType = getImpl().getSeverityReasonType(); | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 | ||||||
| boolean promiseRejection = REASON_PROMISE_REJECTION.equals(severityReasonType); | ||||||
| boolean anr = getImpl().isAnr(this); | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 | ||||||
| if (anr || promiseRejection) { | ||||||
| return DeliveryStrategy.STORE_AND_FLUSH; | ||||||
| } else if (getImpl().isAttemptDeliveryOnCrash()) { | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 | ||||||
| return DeliveryStrategy.STORE_AND_SEND; | ||||||
| } else { | ||||||
| return DeliveryStrategy.STORE_ONLY; | ||||||
| } | ||||||
| } else { | ||||||
| return DeliveryStrategy.SEND_IMMEDIATELY; | ||||||
| } | ||||||
| } | ||||||
|  | ||||||
| /** | ||||||
| * Sets the delivery strategy for this event, which determines how the event | ||||||
| * should be delivered to the Bugsnag API. This allows customization of delivery | ||||||
| * behavior on a per-event basis. | ||||||
| * | ||||||
| * @param deliveryStrategy the delivery strategy to use for this event | ||||||
| */ | ||||||
| public void setDeliveryStrategy(@NonNull DeliveryStrategy deliveryStrategy) { | ||||||
| if (deliveryStrategy != null) { | ||||||
| impl.setDeliveryStrategy(deliveryStrategy); | ||||||
| } else { | ||||||
| logNull("deliveryStrategy"); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| package com.bugsnag.android | ||
|  | ||
| import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Before | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import org.mockito.junit.MockitoJUnitRunner | ||
| import java.util.Date | ||
|  | ||
| @RunWith(MockitoJUnitRunner::class) | ||
| internal class EventDeliveryStrategyTest { | ||
|         
                  YYChen01988 marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
|  | ||
| private val apiKey = "BUGSNAG_API_KEY" | ||
| private val notifier = Notifier() | ||
| private val config = generateImmutableConfig() | ||
| private var testException: RuntimeException? = null | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could reasonably be  | ||
| private val event = Event( | ||
| RuntimeException("Whoops!"), config, | ||
| SeverityReason.newInstance( | ||
| SeverityReason.REASON_UNHANDLED_EXCEPTION | ||
| ), | ||
| NoopLogger | ||
| ) | ||
| 
      Comment on lines
    
      +18
     to 
      +24
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this is only used in  | ||
|  | ||
| @Before | ||
| fun setUp() { | ||
| testException = RuntimeException("Example message") | ||
| event.session = Session( | ||
| "123", Date(), User(null, null, null), false, notifier, NoopLogger, apiKey | ||
| ) | ||
| } | ||
|  | ||
| @Test | ||
| fun testGenerateUnhandledReport() { | ||
| assertEquals(DeliveryStrategy.STORE_ONLY, event.deliveryStrategy) | ||
| event.deliveryStrategy = DeliveryStrategy.SEND_IMMEDIATELY | ||
| assertEquals(DeliveryStrategy.SEND_IMMEDIATELY, event.deliveryStrategy) | ||
| } | ||
|  | ||
| @Test | ||
| fun testANRStoreEventAndFlush() { | ||
| val anrEvent = Event( | ||
| testException, config, | ||
| SeverityReason.newInstance(SeverityReason.REASON_ANR), | ||
| NoopLogger | ||
| ) | ||
| anrEvent.getErrors().get(0).setErrorClass("ANR") | ||
| assertEquals(DeliveryStrategy.STORE_AND_FLUSH, anrEvent.getDeliveryStrategy()) | ||
| } | ||
|  | ||
| @Test | ||
| fun testPromiseRejectionEvetStoreAndFlush() { | ||
| val promiseRejectionEvent = Event( | ||
| testException, config, | ||
| SeverityReason.newInstance(SeverityReason.REASON_PROMISE_REJECTION), | ||
| NoopLogger | ||
| ) | ||
| assertEquals(DeliveryStrategy.STORE_AND_FLUSH, promiseRejectionEvent.getDeliveryStrategy()) | ||
| } | ||
|  | ||
| @Test | ||
| fun testANRWithModifiedErrorClassStoreAndFlush() { | ||
| val modifiedAnrEvent = Event( | ||
| testException, | ||
| config, | ||
| SeverityReason.newInstance(SeverityReason.REASON_PROMISE_REJECTION), | ||
| NoopLogger | ||
| ) | ||
| modifiedAnrEvent.getErrors().get(0).setErrorClass("ANR") | ||
| assertEquals(DeliveryStrategy.STORE_AND_FLUSH, modifiedAnrEvent.getDeliveryStrategy()) | ||
| } | ||
|  | ||
| @Test | ||
| fun testGenerateHandledReport() { | ||
| val state = SeverityReason.newInstance( | ||
| SeverityReason.REASON_HANDLED_EXCEPTION | ||
| ) | ||
| val event = Event(RuntimeException("Whoops!"), config, state, NoopLogger) | ||
| event.session = Session( | ||
| "123", Date(), User(null, null, null), false, notifier, NoopLogger, apiKey | ||
| ) | ||
| assertEquals(DeliveryStrategy.SEND_IMMEDIATELY, event.deliveryStrategy) | ||
| } | ||
|  | ||
| @Test | ||
| fun testDeliveryStrategyStoreAndSend() { | ||
| val configuration = Configuration(apiKey) | ||
| val newConfig = generateImmutableConfig(configuration).apply { | ||
| configuration.setAttemptDeliveryOnCrash(true) | ||
| } | ||
| val unhandledEvent = Event( | ||
| testException, newConfig, | ||
| SeverityReason.newInstance(SeverityReason.REASON_UNHANDLED_EXCEPTION), | ||
| NoopLogger | ||
| ) | ||
| assertEquals(DeliveryStrategy.STORE_ONLY, unhandledEvent.getDeliveryStrategy()) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.