Page MenuHomePhabricator

Don't overwrite original notification with error message notification
ClosedPublic

Authored by marcin on Dec 8 2023, 8:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jul 3, 3:43 PM
Unknown Object (File)
Sun, Jun 30, 4:59 AM
Unknown Object (File)
Sun, Jun 30, 4:24 AM
Unknown Object (File)
Sat, Jun 29, 10:57 PM
Unknown Object (File)
Sat, Jun 29, 3:20 PM
Unknown Object (File)
Tue, Jun 25, 9:39 AM
Unknown Object (File)
Mon, Jun 24, 12:18 AM
Unknown Object (File)
Sun, Jun 23, 11:25 PM
Subscribers

Details

Summary

This differential introduces changes that are supposed to test theory described in: https://linear.app/comm/issue/ENG-4886/nse-error-during-notif-decryption-bad-message-mac#comment-5864be25

Assuming that NSE restarts notification processing even after relevant contentHandler was called then originally displayed notification should overwrite original one. However if instead of using contentHandler to display error message notification we schedule error message notification as local notification then this shouldn't happen.

Before scheduling error message notification we also check if notification with the same isn't already displayed. It should further support theory.

Test Plan
  1. Check that notifications are working correctly, especially that notification collapsing is not broken.
  2. Add artificial throw std::runtime_error(...); a line before we try to decrypt notification, build iOS app and observe logs from NSE in Mac console app. Expect that when you send notification you will see error message instead of regular notification and if you send multiple notifications then in console you will see logs like Got 2 delivered notifications ... (this checks that function isAppShowingNotifWithID actually executes and fetches all delivered notifications).

Unfortunately we are unable to reproduce scenario where NSE actually restarts so to fully test the issue we will need a release and wait until it reoccurs. This testing plan aims to trigger code that this differential introduces and check that they work properly and don't crash.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable