Skip to content

Commit

Permalink
Make several logging improvements for FC
Browse files Browse the repository at this point in the history
- Fix `extraMessage` in `ErrorViewModel`
- Report `pane` in various events
- Add `status` to complete event
  • Loading branch information
tillh-stripe committed Apr 30, 2024
1 parent 6467022 commit ced3750
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 35 deletions.
1 change: 1 addition & 0 deletions financial-connections/detekt-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<ID>LongMethod:AccountItem.kt$@Composable @Preview internal fun AccountItemPreview()</ID>
<ID>LongMethod:Button.kt$@Composable internal fun FinancialConnectionsButton( onClick: () -> Unit, modifier: Modifier = Modifier, type: Type = Primary, size: FinancialConnectionsButton.Size = FinancialConnectionsButton.Size.Regular, enabled: Boolean = true, loading: Boolean = false, content: @Composable (RowScope.() -> Unit) )</ID>
<ID>LongMethod:FinancialConnectionsSheetNativeActivity.kt$FinancialConnectionsSheetNativeActivity$@Composable fun NavHost( initialPane: Pane, testMode: Boolean, )</ID>
<ID>LongMethod:FinancialConnectionsSheetNativeViewModel.kt$FinancialConnectionsSheetNativeViewModel$private fun closeAuthFlow( earlyTerminationCause: EarlyTerminationCause? = null, closeAuthFlowError: Throwable? = null )</ID>
<ID>LongMethod:InstitutionPickerScreen.kt$private fun LazyListScope.searchResults( isInputEmpty: Boolean, payload: Payload, selectedInstitutionId: String?, onInstitutionSelected: (FinancialConnectionsInstitution, Boolean) -> Unit, institutions: Async&lt;InstitutionResponse>, onManualEntryClick: () -> Unit, onSearchMoreClick: () -> Unit )</ID>
<ID>LongMethod:LinkAccountPickerPreviewParameterProvider.kt$LinkAccountPickerPreviewParameterProvider$private fun partnerAccountList()</ID>
<ID>LongMethod:NetworkingSaveToLinkVerificationScreen.kt$@Composable private fun NetworkingSaveToLinkVerificationLoaded( confirmVerificationAsync: Async&lt;Unit>, payload: Payload, onCloseFromErrorClick: (Throwable) -> Unit, onSkipClick: () -> Unit, )</ID>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,18 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
)

class Complete(
pane: Pane,
exception: Throwable?,
exceptionExtraMessage: String?,
connectedAccounts: Int?
connectedAccounts: Int?,
status: String,
) : FinancialConnectionsAnalyticsEvent(
name = "complete",
mapOf(
"pane" to pane.analyticsValue,
"num_linked_accounts" to connectedAccounts?.toString(),
"type" to if (exception == null) "object" else "error"
"type" to if (exception == null) "object" else "error",
"status" to status,
)
.plus(exception?.toEventParams(exceptionExtraMessage) ?: emptyMap())
.filterNotNullValues()
Expand All @@ -89,15 +93,6 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
).filterNotNullValues()
)

class ClickDisconnectLink(
pane: Pane
) : FinancialConnectionsAnalyticsEvent(
name = "click.disconnect_link",
mapOf(
"pane" to pane.analyticsValue,
).filterNotNullValues()
)

class Click(
eventName: String,
pane: Pane
Expand Down Expand Up @@ -171,17 +166,20 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
)

class PollAccountsSucceeded(
pane: Pane,
authSessionId: String,
duration: Long
) : FinancialConnectionsAnalyticsEvent(
name = "polling.accounts.success",
mapOf(
"pane" to pane.analyticsValue,
"authSessionId" to authSessionId,
"duration" to duration.toString(),
).filterNotNullValues()
)

class AccountSelected(
pane: Pane,
selected: Boolean,
isSingleAccount: Boolean,
accountId: String,
Expand All @@ -192,6 +190,7 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
"click.account_picker.account_unselected"
},
mapOf(
"pane" to pane.analyticsValue,
"is_single_account" to isSingleAccount.toString(),
"account" to accountId,
).filterNotNullValues()
Expand All @@ -211,22 +210,26 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
)

class AccountsAutoSelected(
pane: Pane,
accountIds: Set<String>,
isSingleAccount: Boolean
) : FinancialConnectionsAnalyticsEvent(
name = "account_picker.accounts_auto_selected",
mapOf(
"pane" to pane.analyticsValue,
"account_ids" to accountIds.joinToString(" "),
"is_single_account" to isSingleAccount.toString(),
).filterNotNullValues()
)

class PollAttachPaymentsSucceeded(
pane: Pane,
authSessionId: String,
duration: Long
) : FinancialConnectionsAnalyticsEvent(
name = "polling.attachPayment.success",
mapOf(
"pane" to pane.analyticsValue,
"authSessionId" to authSessionId,
"duration" to duration.toString(),
).filterNotNullValues()
Expand Down Expand Up @@ -268,15 +271,6 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
).filterNotNullValues()
)

class VerificationSuccessNoAccounts(
pane: Pane,
) : FinancialConnectionsAnalyticsEvent(
name = "networking.verification.success_no_accounts",
mapOf(
"pane" to pane.analyticsValue,
).filterNotNullValues()
)

class VerificationError(
pane: Pane,
error: Error
Expand All @@ -292,7 +286,6 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
LookupConsumerSession("LookupConsumerSession"),
StartVerificationSessionError("StartVerificationSessionError"),
ConfirmVerificationSessionError("ConfirmVerificationSessionError"),
NetworkedAccountsRetrieveMethodError("NetworkedAccountsRetrieveMethodError"),
MarkLinkVerifiedError("MarkLinkVerifiedError"),
}
}
Expand Down Expand Up @@ -366,15 +359,20 @@ internal sealed class FinancialConnectionsAnalyticsEvent(
includePrefix = false,
)

class AuthSessionUrlReceived(url: String, status: String, authSessionId: String?) :
FinancialConnectionsAnalyticsEvent(
name = "auth_session.url_received",
params = mapOf(
"status" to status,
"url" to url,
"auth_session_id" to (authSessionId ?: "")
).filterNotNullValues(),
)
class AuthSessionUrlReceived(
pane: Pane,
url: String,
status: String,
authSessionId: String?
) : FinancialConnectionsAnalyticsEvent(
name = "auth_session.url_received",
params = mapOf(
"pane" to pane.analyticsValue,
"status" to status,
"url" to url,
"auth_session_id" to (authSessionId ?: "")
).filterNotNullValues(),
)

class AuthSessionRetrieved(nextPane: Pane, authSessionId: String) :
FinancialConnectionsAnalyticsEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ internal class NativeAuthFlowCoordinator @Inject constructor() {
data class Complete(
val cause: EarlyTerminationCause? = null
) : Message {
enum class EarlyTerminationCause(val value: String) {
USER_INITIATED_WITH_CUSTOM_MANUAL_ENTRY("user_initiated_with_custom_manual_entry")
enum class EarlyTerminationCause(
val value: String,
val analyticsValue: String,
) {
USER_INITIATED_WITH_CUSTOM_MANUAL_ENTRY(
value = "user_initiated_with_custom_manual_entry",
analyticsValue = "custom_manual_entry"
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
if (partnerAccountList.data.isNotEmpty()) {
eventTracker.track(
PollAccountsSucceeded(
pane = PANE,
authSessionId = activeAuthSession.id,
duration = millis
)
Expand Down Expand Up @@ -156,6 +157,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
val selectedId = setOfNotNull(payload.selectableAccounts.firstOrNull()?.id)
eventTracker.track(
AccountsAutoSelected(
pane = PANE,
isSingleAccount = true,
accountIds = selectedId
)
Expand All @@ -168,6 +170,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
val selectedIds = payload.selectableAccounts.map { it.id }.toSet()
eventTracker.track(
AccountsAutoSelected(
pane = PANE,
isSingleAccount = false,
accountIds = selectedIds
)
Expand Down Expand Up @@ -236,6 +239,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
if (newIds.size == 1) {
eventTracker.track(
AccountSelected(
pane = PANE,
isSingleAccount = isSingleAccount,
selected = true,
accountId = newIds.first()
Expand All @@ -245,6 +249,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
if (removedIds.size == 1) {
eventTracker.track(
AccountSelected(
pane = PANE,
isSingleAccount = isSingleAccount,
selected = false,
accountId = removedIds.first()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ internal class AttachPaymentViewModel @AssistedInject constructor(
}
eventTracker.track(
PollAttachPaymentsSucceeded(
pane = PANE,
authSessionId = authSession.id,
duration = millis
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ internal class ErrorViewModel @AssistedInject constructor(
ErrorState::payload,
onFail = { error ->
eventTracker.logError(
extraMessage = "Error linking more accounts",
extraMessage = "Error loading the error screen payload",
error = error,
logger = logger,
pane = PANE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ internal class LinkAccountPickerViewModel @AssistedInject constructor(
val isNewSelection = partnerAccount.id !in state.selectedAccountIds

val event = FinancialConnectionsAnalyticsEvent.AccountSelected(
pane = PANE,
selected = isNewSelection,
isSingleAccount = payload.singleAccount,
accountId = partnerAccount.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ internal class PartnerAuthViewModel @AssistedInject constructor(
val authSession = getOrFetchSync().manifest.activeAuthSession
eventTracker.track(
AuthSessionUrlReceived(
pane = PANE,
url = url,
authSessionId = authSession?.id,
status = "failed"
Expand Down Expand Up @@ -309,6 +310,7 @@ internal class PartnerAuthViewModel @AssistedInject constructor(
val authSession = manifest.activeAuthSession
eventTracker.track(
AuthSessionUrlReceived(
pane = PANE,
url = url ?: "none",
authSessionId = authSession?.id,
status = "cancelled"
Expand Down Expand Up @@ -381,6 +383,7 @@ internal class PartnerAuthViewModel @AssistedInject constructor(
val authSession = getOrFetchSync().manifest.activeAuthSession
eventTracker.track(
AuthSessionUrlReceived(
pane = PANE,
url = url,
authSessionId = authSession?.id,
status = "success"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,14 @@ internal class FinancialConnectionsSheetNativeViewModel @Inject constructor(
setState { copy(completed = true) }
runCatching {
val session = completeFinancialConnectionsSession(earlyTerminationCause?.value)
val status = computeSessionCompletionStatus(session, earlyTerminationCause, closeAuthFlowError)
eventTracker.track(
Complete(
pane = currentPane.value,
exception = null,
exceptionExtraMessage = null,
connectedAccounts = session.accounts.data.count()
connectedAccounts = session.accounts.data.count(),
status = status,
)
)
when {
Expand Down Expand Up @@ -340,16 +343,26 @@ internal class FinancialConnectionsSheetNativeViewModel @Inject constructor(
logger.error(errorMessage, completeSessionError)
eventTracker.track(
Complete(
pane = currentPane.value,
exception = completeSessionError,
exceptionExtraMessage = errorMessage,
connectedAccounts = null
connectedAccounts = null,
status = "failed",
)
)
finishWithResult(Failed(closeAuthFlowError ?: completeSessionError))
}
}
}

private fun computeSessionCompletionStatus(
session: FinancialConnectionsSession,
earlyTerminationCause: EarlyTerminationCause?,
closeAuthFlowError: Throwable?,
): String {
return earlyTerminationCause?.analyticsValue ?: session.completionStatus(closeAuthFlowError)
}

private fun finishWithResult(
result: FinancialConnectionsSheetActivityResult
) {
Expand Down Expand Up @@ -567,3 +580,16 @@ private fun FinancialConnectionsSheetNativeState.toTopAppBarState(
isTestMode = testMode,
)
}

private fun FinancialConnectionsSession.completionStatus(
closeAuthFlowError: Throwable?,
): String {
val completed = accounts.data.isNotEmpty() || paymentAccount != null || bankAccountToken != null
return if (completed) {
"completed"
} else if (closeAuthFlowError != null) {
"failed"
} else {
"canceled"
}
}

0 comments on commit ced3750

Please sign in to comment.