Skip to content
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

Make several logging improvements for FC #8380

Merged
merged 2 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -12,12 +12,45 @@ internal class CompleteFinancialConnectionsSession @Inject constructor(
) {

suspend operator fun invoke(
terminalError: String? = null
): FinancialConnectionsSession {
earlyTerminationCause: NativeAuthFlowCoordinator.Message.Complete.EarlyTerminationCause?,
closeAuthFlowError: Throwable?,
): Result {
val session = repository.postCompleteFinancialConnectionsSessions(
terminalError = terminalError,
clientSecret = configuration.financialConnectionsSessionClientSecret
terminalError = earlyTerminationCause?.value,
clientSecret = configuration.financialConnectionsSessionClientSecret,
)
return fetchPaginatedAccountsForSession(session)

val fullSession = fetchPaginatedAccountsForSession(session)

return Result(
session = fullSession,
status = computeSessionCompletionStatus(fullSession, earlyTerminationCause, closeAuthFlowError),
)
}

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

data class Result(
val session: FinancialConnectionsSession,
val status: String,
)
}

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"
}
}
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 @@ -99,6 +99,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
if (partnerAccountList.data.isNotEmpty()) {
eventTracker.track(
PollAccountsSucceeded(
pane = PANE,
authSessionId = activeAuthSession.id,
duration = millis
)
Expand Down Expand Up @@ -157,6 +158,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
val selectedId = setOfNotNull(payload.selectableAccounts.firstOrNull()?.id)
eventTracker.track(
AccountsAutoSelected(
pane = PANE,
isSingleAccount = true,
accountIds = selectedId
)
Expand All @@ -169,6 +171,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 @@ -237,6 +240,7 @@ internal class AccountPickerViewModel @AssistedInject constructor(
if (newIds.size == 1) {
eventTracker.track(
AccountSelected(
pane = PANE,
isSingleAccount = isSingleAccount,
selected = true,
accountId = newIds.first()
Expand All @@ -246,6 +250,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 @@ -76,6 +76,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 @@ -68,7 +68,7 @@ internal class ErrorViewModel @AssistedInject constructor(
ErrorState::payload,
onFail = { error ->
eventTracker.logError(
extraMessage = "Error linking more accounts",
extraMessage = "Error loading the error screen payload",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🥴

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 @@ -274,6 +274,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 @@ -314,6 +315,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 @@ -388,6 +390,7 @@ internal class PartnerAuthViewModel @AssistedInject constructor(
).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 @@ -295,12 +295,15 @@ internal class FinancialConnectionsSheetNativeViewModel @Inject constructor(
if (stateFlow.value.completed) return@launch
setState { copy(completed = true) }
runCatching {
val session = completeFinancialConnectionsSession(earlyTerminationCause?.value)
val completionResult = completeFinancialConnectionsSession(earlyTerminationCause, closeAuthFlowError)
val session = completionResult.session
eventTracker.track(
Complete(
pane = currentPane.value,
exception = null,
exceptionExtraMessage = null,
connectedAccounts = session.accounts.data.count()
connectedAccounts = session.accounts.data.count(),
status = completionResult.status,
)
)
when {
Expand Down Expand Up @@ -340,9 +343,11 @@ 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))
Expand Down