fix: invalid cookie data in NSE - WPB-24202#4734
Open
samwyndham wants to merge 16 commits into
Open
Conversation
samwyndham
requested review from
johnxnguyen and
netbe
and removed request for
netbe
Contributor
Test Results2 173 tests 2 146 ✅ 2m 11s ⏱️ Results for commit 83d8f54. ♻️ This comment has been updated with latest results. Summary: workflow run #26153092219 |
johnxnguyen
approved these changes
May 19, 2026
Comment on lines
+109
to
+112
| WireLogger.notifications.critical( | ||
| "no cookie encryption key, not loading service", | ||
| attributes: .safePublic | ||
| ) |
Collaborator
There was a problem hiding this comment.
suggestion: maybe it's worth stating in the log that this may be expected if the phone restarted and hasn't been unlocked yet.
Contributor
Author
There was a problem hiding this comment.
I changed it to warn level as it may not exactly be an error but might surprise users.
|
johnxnguyen
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Some users are experiencing an issue with notifications not being delivered. Looking through their logs we see repeated issues with
invalidCookieData.In our code base this means that we have fetched the cookie data from the keychain but we fail to convert this to cookie data using the NSKeyedUnarchiver. The data should be in the
bplistformat but it is not (see my comment in the ticket for more info).From testing this is the exact behavior we would see if we tried to decrypt the cookie with an incorrect cookie key. However strangely this issue only affects the NSE not the main app which both use the same cookie key stored in shared user defaults. The cookie key is created lazily if user defaults doesn't already have one set.
Searching online I found cases where UserDefaults would return nil for an already set value and the best guess at the cause was related to permissions - UserDefaults being accessed before the device was first unlocked after a restart.
In the logs for a user I looked at this happens over and over again. Additionally of note we also have this line for each time a notification is received with the exact same process id:
So it is possible that these repeated failures are all happening in the same process.
A hypothesis
This PR attempts to do a possible fix of the following hypothesis that I have not managed to reproduce.
The fix here is to fail early if we don't have an existing cookie key.
Testing
Ensure that push notifications are still working.
Checklist
[WPB-XXX].