Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

Added DeliveryStrategy to event delivery #2312

wants to merge 1 commit into from

Conversation

@YYChen01988
Copy link
Contributor

@YYChen01988 YYChen01988 commented Oct 8, 2025 •
edited by lemnik
Loading

Goal

Introduced DeliveryStrategy to Event

Changeset

Changing the Event.deliveryStrategy will alter how Bugsnag attempts to deliver the Event independent of the unhandled status.

Testing

Updated existing tests

@YYChen01988 YYChen01988 requested a review from lemnik as a code owner October 8, 2025 08:22
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Oct 8, 2025 •
edited
Loading

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1838.28 1603.0
arm64_v8a 647.43 409.86
armeabi_v7a 590.09 352.52
x86 721.14 483.57
x86_64 692.47 454.9

Generated by 🚫 Danger

@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch from cfff60b to f3befb5 Compare October 8, 2025 09:44
lemnik
lemnik reviewed Oct 8, 2025
View reviewed changes
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch 3 times, most recently from 0196aaf to c9adb65 Compare October 8, 2025 11:56
@YYChen01988 YYChen01988 requested a review from lemnik October 8, 2025 11:57
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch from c9adb65 to aede41d Compare October 8, 2025 12:06
lemnik
lemnik reviewed Oct 20, 2025
View reviewed changes
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch 5 times, most recently from 04fb4e9 to 1cd1c36 Compare October 21, 2025 14:46
@YYChen01988 YYChen01988 requested a review from lemnik October 21, 2025 14:46
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch from 1cd1c36 to fae2f5b Compare October 21, 2025 15:12
lemnik
lemnik requested changes Oct 22, 2025
View reviewed changes
bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryDelegateTest.kt Outdated
Comment on lines 77 to 78
event.session =
Session("123", Date(), User(null, null, null), false, notifier, NoopLogger, apiKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: formatting

@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch 2 times, most recently from d28b8e6 to 63a16c1 Compare October 24, 2025 10:50
@YYChen01988 YYChen01988 requested a review from lemnik October 24, 2025 10:54
lemnik
lemnik reviewed Oct 24, 2025
View reviewed changes
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch from 63a16c1 to 662cb33 Compare October 29, 2025 11:49
@YYChen01988 YYChen01988 requested a review from lemnik October 29, 2025 11:50
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch 3 times, most recently from 3ba6054 to 3410976 Compare October 29, 2025 15:23
@YYChen01988 YYChen01988 force-pushed the PLAT-14976/implement-DeliveryStrategy-to-Event branch from 3410976 to 02bfa1d Compare October 29, 2025 15:24
lemnik
lemnik reviewed Oct 30, 2025
View reviewed changes
bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java
return impl.getDeliveryStrategy();
}

if (getImpl().getOriginalUnhandled()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (getImpl().getOriginalUnhandled()) {
if (impl.getOriginalUnhandled()) {

bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java
}

if (getImpl().getOriginalUnhandled()) {
String severityReasonType = getImpl().getSeverityReasonType();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
String severityReasonType = getImpl().getSeverityReasonType();
String severityReasonType = impl.getSeverityReasonType();

bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java
if (getImpl().getOriginalUnhandled()) {
String severityReasonType = getImpl().getSeverityReasonType();
boolean promiseRejection = REASON_PROMISE_REJECTION.equals(severityReasonType);
boolean anr = getImpl().isAnr(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
boolean anr = getImpl().isAnr(this);
boolean anr = impl.isAnr(this);

bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java
boolean anr = getImpl().isAnr(this);
if (anr || promiseRejection) {
return DeliveryStrategy.STORE_AND_FLUSH;
} else if (getImpl().isAttemptDeliveryOnCrash()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (getImpl().isAttemptDeliveryOnCrash()) {
} else if (impl.isAttemptDeliveryOnCrash()) {

bugsnag-android-core/src/test/java/com/bugsnag/android/EventDeliveryStrategyTest.kt
Comment on lines +18 to +24
private val event = Event(
RuntimeException("Whoops!"), config,
SeverityReason.newInstance(
SeverityReason.REASON_UNHANDLED_EXCEPTION
),
NoopLogger
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is only used in testGenerateUnhandledReport it should be moved there

bugsnag-android-core/src/test/java/com/bugsnag/android/EventDeliveryStrategyTest.kt
private val apiKey = "BUGSNAG_API_KEY"
private val notifier = Notifier()
private val config = generateImmutableConfig()
private var testException: RuntimeException? = null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could reasonably be lateinit var instead of being nullable? Alternatively it could be replaced with fun createTestException()

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

Reviewers

@lemnik lemnik lemnik requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@YYChen01988 @bugsnagbot @lemnik