Skip to content

Add extra logs the 'send call notification' flow #4819

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

Merged
merged 5 commits into from
Jun 5, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,19 @@

sealed interface CallType : NodeInputs, Parcelable {
@Parcelize
data class ExternalUrl(val url: String) : CallType
data class ExternalUrl(val url: String) : CallType {
override fun toString(): String {
return "ExternalUrl"
}
}

Check warning on line 22 in features/call/api/src/main/kotlin/io/element/android/features/call/api/CallType.kt

View check run for this annotation

Codecov / codecov/patch

features/call/api/src/main/kotlin/io/element/android/features/call/api/CallType.kt#L22

Added line #L22 was not covered by tests

@Parcelize
data class RoomCall(
val sessionId: SessionId,
val roomId: RoomId,
) : CallType
) : CallType {
override fun toString(): String {
return "RoomCall(sessionId=$sessionId, roomId=$roomId)"
}
}

Check warning on line 32 in features/call/api/src/main/kotlin/io/element/android/features/call/api/CallType.kt

View check run for this annotation

Codecov / codecov/patch

features/call/api/src/main/kotlin/io/element/android/features/call/api/CallType.kt#L32

Added line #L32 was not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import kotlinx.serialization.json.contentOrNull
import kotlinx.serialization.json.jsonObject
import kotlinx.serialization.json.jsonPrimitive
import timber.log.Timber
import java.util.UUID

class CallScreenPresenter @AssistedInject constructor(
Expand Down Expand Up @@ -211,6 +212,7 @@
theme = theme,
).getOrThrow()
callWidgetDriver.value = result.driver
Timber.d("Call widget driver initialized for sessionId: ${inputs.sessionId}, roomId: ${inputs.roomId}")
result.url
}
}
Expand All @@ -221,10 +223,12 @@
private fun HandleMatrixClientSyncState() {
val coroutineScope = rememberCoroutineScope()
DisposableEffect(Unit) {
val client = (callType as? CallType.RoomCall)?.sessionId?.let {
matrixClientsProvider.getOrNull(it)
} ?: return@DisposableEffect onDispose { }
val roomCallType = callType as? CallType.RoomCall ?: return@DisposableEffect onDispose {}
val client = matrixClientsProvider.getOrNull(roomCallType.sessionId) ?: return@DisposableEffect onDispose {
Timber.w("No MatrixClient found for sessionId, can't send call notification: ${roomCallType.sessionId}")
}
coroutineScope.launch {
Timber.d("Observing sync state in-call for sessionId: ${roomCallType.sessionId}")
client.syncService().syncState
.collect { state ->
if (state == SyncState.Running) {
Expand All @@ -235,19 +239,37 @@
}
}
onDispose {
Timber.d("Stopped observing sync state in-call for sessionId: ${roomCallType.sessionId}")
// Make sure we mark the call as ended in the app state
appForegroundStateService.updateIsInCallState(false)
}
}
}

private suspend fun MatrixClient.notifyCallStartIfNeeded(roomId: RoomId) {
if (!notifiedCallStart) {
val activeRoomForSession = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId)
val sendCallNotificationResult = activeRoomForSession?.sendCallNotificationIfNeeded()
?: getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() }
sendCallNotificationResult?.onSuccess { notifiedCallStart = true }
if (notifiedCallStart) return

val activeRoomForSession = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId)
val sendCallNotificationResult = if (activeRoomForSession != null) {
Timber.d("Notifying call start for room $roomId. Has room call: ${activeRoomForSession.info().hasRoomCall}")
activeRoomForSession.sendCallNotificationIfNeeded()

Check warning on line 255 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt#L254-L255

Added lines #L254 - L255 were not covered by tests
} else {
// Instantiate the room from the session and roomId and send the notification
getJoinedRoom(roomId)?.use { room ->
Timber.d("Notifying call start for room $roomId. Has room call: ${room.info().hasRoomCall}")
room.sendCallNotificationIfNeeded()
} ?: run {
Timber.w("No room found for session $sessionId and room $roomId, skipping call notification.")

Check warning on line 262 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt#L261-L262

Added lines #L261 - L262 were not covered by tests
return
}
}

sendCallNotificationResult.fold(
onSuccess = { notifiedCallStart = true },
onFailure = { error ->
Timber.e(error, "Failed to send call notification for room $roomId.")
}

Check warning on line 271 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt#L270-L271

Added lines #L270 - L271 were not covered by tests
)
}

private fun parseMessage(message: String): WidgetMessage? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,13 @@
AsyncData.Uninitialized,
is AsyncData.Loading ->
ProgressDialog(text = stringResource(id = CommonStrings.common_please_wait))
is AsyncData.Failure ->
is AsyncData.Failure -> {
Timber.e(state.urlState.error, "WebView failed to load URL: ${state.urlState.error.message}")
ErrorDialog(
content = state.urlState.error.message.orEmpty(),
onSubmit = { state.eventSink(CallScreenEvents.Hangup) },
)
}
is AsyncData.Success -> Unit
}
}
Expand Down Expand Up @@ -242,6 +244,20 @@
ConsoleMessage.MessageLevel.WARNING -> Log.WARN
else -> Log.DEBUG
}

val message = buildString {
append(consoleMessage.sourceId())
append(":")
append(consoleMessage.lineNumber())
append(" ")
append(consoleMessage.message())
}

Check warning on line 254 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt#L248-L254

Added lines #L248 - L254 were not covered by tests

if (message.contains("password=")) {
// Avoid logging any messages that contain "password" to prevent leaking sensitive information
return true

Check warning on line 258 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt#L258

Added line #L258 was not covered by tests
}

Timber.tag("WebView").log(
priority = priority,
message = buildString {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@

pictureInPicturePresenter.setPipView(this)

Timber.d("Created ElementCallActivity with call type: ${webViewTarget.value}")

Check warning on line 100 in features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt

View check run for this annotation

Codecov / codecov/patch

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt#L100

Added line #L100 was not covered by tests

setContent {
val pipState = pictureInPicturePresenter.present()
ListenToAndroidEvents(pipState)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ class DefaultActiveCallManager @Inject constructor(
Timber.tag(tag).w("Call type $callType does not match the active call type, ignoring")
return
}

Timber.tag(tag).d("Hung up call: $callType")
Copy link
Member

Choose a reason for hiding this comment

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

Is there some sensitive information in callType? Maybe safer to shrink some data first?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there's anything there that won't be logged at some other point (URL in webview or session id + room id for room calls).

Copy link
Member Author

Choose a reason for hiding this comment

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

I added some changes to prevent the URL from being logged.


cancelIncomingCallNotification()
if (activeWakeLock?.isHeld == true) {
Timber.tag(tag).d("Releasing partial wakelock after hang up")
Expand All @@ -191,6 +194,8 @@ class DefaultActiveCallManager @Inject constructor(
}

override suspend fun joinedCall(callType: CallType) = mutex.withLock {
Timber.tag(tag).d("Joined call: $callType")

cancelIncomingCallNotification()
if (activeWakeLock?.isHeld == true) {
Timber.tag(tag).d("Releasing partial wakelock after joining call")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,18 @@ class RoomCallStatePresenter @Inject constructor(
(currentCall as? CurrentCall.RoomCall)?.roomId == room.roomId
}
}
val callState = when {
isAvailable.not() -> RoomCallState.Unavailable
roomInfo.hasRoomCall -> RoomCallState.OnGoing(
canJoinCall = canJoinCall,
isUserInTheCall = isUserInTheCall,
isUserLocallyInTheCall = isUserLocallyInTheCall,
)
else -> RoomCallState.StandBy(canStartCall = canJoinCall)
val callState by remember {
derivedStateOf {
when {
isAvailable.not() -> RoomCallState.Unavailable
roomInfo.hasRoomCall -> RoomCallState.OnGoing(
canJoinCall = canJoinCall,
isUserInTheCall = isUserInTheCall,
isUserLocallyInTheCall = isUserLocallyInTheCall,
)
else -> RoomCallState.StandBy(canStartCall = canJoinCall)
}
}
}
return callState
}
Expand Down
Loading