fix: crash calleventstatus - WPB-10449 #2233

netbe · 2024-12-02T14:50:09Z

WPB-10449 Crash on ZMTimer - CallEventStatus

Issue

There is a crash (see ticket) involving CallEventStatus that happens from time to time. Looking at the usage of CallEventStatus, it appears we don't really need this at all. This PR cleans up unused code in order to resolve the crash.

Testing

running app and check for regression

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

netbe · 2024-12-02T14:50:56Z

wire-ios-sync-engine/Source/UserSession/ZMUserSession/ZMUserSession.swift

@@ -1037,12 +1037,13 @@ extension ZMUserSession: ZMSyncStateDelegate {
        WireLogger.updateEvent.info("process pending call events")
        Task {
            do {
+                // why not processing only the call events (should be stored here?)


question maybe to @johnxnguyen

I guess this is an optim that could be done in another PR, to directly process callEvents instead of buffered ones. I don't see why we would need to process all events in a method processPendingCallEvents

I'm not sure why it does this... but it does more than just process events, it also decrypts and stores first the buffered events that may have be received in the push channel. If you were going to optimize this, you need to decrypt and store the buffered events, and then process only the stored calling events.

added to a ticket for it

netbe · 2024-12-02T14:52:14Z

wire-ios-sync-engine/Source/Notifications/VoIPPushManager.swift

@@ -182,7 +182,8 @@ public final class VoIPPushManager: NSObject, PKPushRegistryDelegate {
        payload: [AnyHashable: Any],
        completion: @escaping () -> Void
    ) {
-        Self.logger.trace("process voIP push, payload: \(payload)")
+        // TODO: check this is dead code right?


from my understanding this is never used, we always use the APNS code, is that right?

The device will only receive APNS pushes and wake the NSE, but the NSE can create a "fake voip push" when it detects a call event and pass it to the main app.

From the point of view of the app, it looks like a real VoIP push, but in reality it came from the NSE. So we need this code.

@samwyndham you're referring to this?

Yes that is what I'm referring to

I was mistaken here which we clarified in a call a while ago.

cc @samwyndham

github-actions · 2024-12-02T14:59:08Z

Test Results

3 files 3 suites 2m 21s ⏱️
2 051 tests 2 048 ✅ 2 💤 1 ❌
2 051 runs 2 049 ✅ 2 💤 0 ❌

For more details on these failures, see this check.

Results for commit a921213.

♻️ This comment has been updated with latest results.

wire-ios-sync-engine/Source/Synchronization/ZMOperatonLoop+Background.swift

wire-ios-sync-engine/Source/Synchronization/Strategies/CallingRequestStrategy.swift

wire-ios-sync-engine/Source/Notifications/VoIPPushManager.swift

samwyndham

Did a first pass and looks good but I guess we need to resolve @johnxnguyens points first and maybe add back some of the code that was removed?

johnxnguyen · 2025-01-13T15:54:23Z

wire-ios-sync-engine/Source/UserSession/ZMUserSession/ZMUserSession.swift

@@ -1037,12 +1037,13 @@ extension ZMUserSession: ZMSyncStateDelegate {
        WireLogger.updateEvent.info("process pending call events")
        Task {
            do {
+                // why not processing only the call events (should be stored here?)


I'm not sure why it does this... but it does more than just process events, it also decrypts and stores first the buffered events that may have be received in the push channel. If you were going to optimize this, you need to decrypt and store the buffered events, and then process only the stored calling events.

datadog-wireapp · 2025-01-15T13:03:44Z

Datadog Report

Branch report: fix/crash-calleventstatus
Commit report: 5e8be44
Test service: wire-ios-mono

✅ 0 Failed, 2014 Passed, 2 Skipped, 2m 6.69s Total Time

samwyndham

Nice work 🛁!

netbe self-assigned this Dec 2, 2024

netbe marked this pull request as draft December 2, 2024 14:50

github-actions bot added the Fix PR Title 🤦‍♂️ label Dec 2, 2024

netbe commented Dec 2, 2024

View reviewed changes

netbe changed the title ~~WIP - fix crash calleventstatus~~ fix: crash calleventstatus - WPB-10449 Dec 20, 2024

echoes-hq bot added the echoes: unplanned Any work item that isn’t part of the product or technical roadmap. label Dec 20, 2024

github-actions bot removed the Fix PR Title 🤦‍♂️ label Dec 20, 2024

netbe changed the base branch from release/cycle-3.114 to release/cycle-3.115 December 20, 2024 08:43

netbe changed the base branch from release/cycle-3.115 to develop December 20, 2024 08:44

netbe added 4 commits December 20, 2024 16:11

wip

f6f5fac

wip

f10785f

remove unused code

23b7598

remove callback for avs - not used

b1b0aed

netbe force-pushed the fix/crash-calleventstatus branch from db256ac to b1b0aed Compare December 20, 2024 15:12

netbe added 5 commits January 7, 2025 22:12

fix missing import

c3b5ace

use dedicated queue instead of mainQueue

ff54f74

format

da80d11

remove obsolete tests

53568af

remove callEventStatus

febbcf2

netbe force-pushed the fix/crash-calleventstatus branch from bf0e86f to febbcf2 Compare January 9, 2025 13:59

fix conflict

714f883

revert

cc971ab

netbe commented Jan 9, 2025

View reviewed changes

wire-ios-sync-engine/Source/Synchronization/ZMOperatonLoop+Background.swift Show resolved Hide resolved

netbe commented Jan 9, 2025

View reviewed changes

wire-ios-sync-engine/Source/Synchronization/Strategies/CallingRequestStrategy.swift Show resolved Hide resolved

netbe commented Jan 9, 2025

View reviewed changes

wire-ios-sync-engine/Source/Notifications/VoIPPushManager.swift Show resolved Hide resolved

netbe marked this pull request as ready for review January 9, 2025 14:07

netbe requested review from a team, samwyndham, caldrian and KaterinaWire and removed request for a team January 9, 2025 14:07

samwyndham reviewed Jan 10, 2025

View reviewed changes

netbe requested a review from johnxnguyen January 13, 2025 15:39

johnxnguyen approved these changes Jan 13, 2025

View reviewed changes

add followup ticket

8adc1a3

netbe requested a review from samwyndham January 14, 2025 15:37

Merge branch 'develop' into fix/crash-calleventstatus

72d617d

netbe enabled auto-merge January 14, 2025 15:38

remove callEnvent

2677914

fix

a921213

samwyndham approved these changes Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: crash calleventstatus - WPB-10449 #2233

fix: crash calleventstatus - WPB-10449 #2233

netbe commented Dec 2, 2024 •

edited

Loading

netbe Dec 2, 2024

netbe Jan 9, 2025

johnxnguyen Jan 13, 2025

netbe Jan 14, 2025

netbe Dec 2, 2024

johnxnguyen Dec 6, 2024

johnxnguyen Dec 6, 2024

netbe Jan 10, 2025

samwyndham Jan 10, 2025

johnxnguyen Jan 13, 2025

netbe Jan 14, 2025

github-actions bot commented Dec 2, 2024 •

edited

Loading

samwyndham left a comment

johnxnguyen Jan 13, 2025

datadog-wireapp bot commented Jan 15, 2025 •

edited

Loading

samwyndham left a comment

fix: crash calleventstatus - WPB-10449 #2233

Are you sure you want to change the base?

fix: crash calleventstatus - WPB-10449 #2233

Conversation

netbe commented Dec 2, 2024 • edited Loading

Issue

Testing

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 2, 2024 • edited Loading

Test Results

samwyndham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datadog-wireapp bot commented Jan 15, 2025 • edited Loading

Datadog Report

samwyndham left a comment

Choose a reason for hiding this comment

netbe commented Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 2, 2024 •

edited

Loading

datadog-wireapp bot commented Jan 15, 2025 •

edited

Loading