-
Notifications
You must be signed in to change notification settings - Fork 54
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
Add offline entitlements diagnostics events #1671
Conversation
name = DiagnosticsEntryName.ERROR_ENTERING_OFFLINE_ENTITLEMENTS_MODE, | ||
properties = mapOf( | ||
"offline_entitlement_error_reason" to reason, | ||
"error_message" to "${error.message}. Underlying error: ${error.underlyingErrorMessage}", |
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.
This won't actually be tracked in Grafana, but I thought it would still be useful to send it.
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.
at the very least it'll be useful in elastic if we're looking through logs of a single customer
|
||
fun trackErrorEnteringOfflineEntitlementsMode(error: PurchasesError) { | ||
val isOneTimePurchaseFoundError = error.code == PurchasesErrorCode.UnsupportedError && | ||
error.underlyingErrorMessage == OfflineEntitlementsStrings.OFFLINE_ENTITLEMENTS_UNSUPPORTED_INAPP_PURCHASES |
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.
I thought about making this a parameter instead of relying on processing the error... But this cleans it up a bit IMO.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1671 +/- ##
==========================================
- Coverage 83.38% 83.38% -0.01%
==========================================
Files 220 220
Lines 7486 7510 +24
Branches 1053 1059 +6
==========================================
+ Hits 6242 6262 +20
Misses 830 830
- Partials 414 418 +4 ☔ View full report in Codecov by Sentry. |
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.
looking good, minor comments
name = DiagnosticsEntryName.ERROR_ENTERING_OFFLINE_ENTITLEMENTS_MODE, | ||
properties = mapOf( | ||
"offline_entitlement_error_reason" to reason, | ||
"error_message" to "${error.message}. Underlying error: ${error.underlyingErrorMessage}", |
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.
at the very least it'll be useful in elastic if we're looking through logs of a single customer
purchases/src/test/java/com/revenuecat/purchases/common/diagnostics/DiagnosticsTrackerTest.kt
Outdated
Show resolved
Hide resolved
fun `trackErrorEnteringOfflineEntitlementsMode tracks correct data for one time purchase error`() { | ||
val expectedProperties = mapOf( | ||
"offline_entitlement_error_reason" to "one_time_purchase_found", | ||
"error_message" to "There was a problem with the operation. Looks like we doesn't support that yet. Check the underlying error for more details.. Underlying error: Offline entitlements are not supported for active inapp purchases. Found active inapp purchases.", |
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.
"error_message" to "There was a problem with the operation. Looks like we doesn't support that yet. Check the underlying error for more details.. Underlying error: Offline entitlements are not supported for active inapp purchases. Found active inapp purchases.", | |
"error_message" to "There was a problem with the operation. Looks like we don't support that yet. Check the underlying error for more details. Underlying error: Offline entitlements are not supported for active inapp purchases. Found active inapp purchases.", |
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.
I'd also skip the "looks like we don't support that yet" bit entirely, I'm not sure we'll ever be able to support one-times in the first place.
Also... I guess active inapp purchases is technically correct, but like, that sounds like any purchases, right? I'd maybe change the copy to one time purchases
for clarity
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.
it'd be great to add a link to documentation too, we usually use the Rebrandly links for that kind of thing. It helps in case we ever want to document this very specifically, if we have a Rebrandly in place we can update where it points to later on
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.
I'd also skip the "looks like we don't support that yet" bit entirely
That part is actually more generic, and it's the error message we display every time we get the UnsupportedError
code... I could change it, but I would prefer to leave that part as is, since it might apply to something else... But happy to revamp this in our future better error handling exploration!
4e4a513
to
d5f5920
Compare
**This is an automatic release.** ### New Features * Paywalls: Allow closed button color to be configured (#1674) via Josh Holtz (@joshdholtz) ### Other Changes * Add offline entitlements diagnostics events (#1671) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <[email protected]>
Description
This adds some new events to get some data into offline entitlements when diagnostics is enabled:
entered_offline_entitlements_mode
error_entering_offline_entitlements_mode