Skip to content

fix(apple): Persist last notified version#8122

Merged
jamilbk merged 2 commits intomainfrom
fix/swift-last-dismissed
Feb 14, 2025
Merged

fix(apple): Persist last notified version#8122
jamilbk merged 2 commits intomainfrom
fix/swift-last-dismissed

Conversation

@jamilbk
Copy link
Copy Markdown
Member

@jamilbk jamilbk commented Feb 13, 2025

Notifications on Apple platforms are delivered with best-effort reliability and are not guaranteed.

They can also be queued up by the system so that, for example, it's possible to issue a notification, quit the app, and then upon the next launch of the app, receive the notification.

In this second case, if the user dismissed the notification, we will crash. This is because we only track the lastNotifiedVersion in the NotificationAdapter instance object and don't persist it to disk, then we assert the value not to be nil when saving the user's dismiss action.

To fix this, we persist the lastNotifiedVersion to the UserDefaults store and attempt to read this when the user is dismissing the notification. If we can't read it for some reason, we still dismiss the notification but won't prevent showing it again on the next update check.

A minor bug is also fixed where the original author didn't correctly call the function's completionHandler. Also, unused instance vars lastDismissedVersion left over from the original author are removed as well.

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2025 10:31pm

@jamilbk jamilbk force-pushed the fix/swift-last-dismissed branch from c2589f2 to ce90e36 Compare February 13, 2025 15:30
@sentry
Copy link
Copy Markdown

sentry bot commented Feb 13, 2025

Sentry Issue: APPLE-CLIENT-2B

@jamilbk jamilbk changed the title fix(apple): Persist last notified and last dismissed versions fix(apple): Persist last notified version Feb 13, 2025
Copy link
Copy Markdown
Member

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Worth a changelog entry?

@jamilbk jamilbk force-pushed the fix/swift-last-dismissed branch from ce90e36 to 0798e5f Compare February 13, 2025 22:29
@jamilbk jamilbk added this pull request to the merge queue Feb 13, 2025
Merged via the queue into main with commit e23bd97 Feb 14, 2025
@jamilbk jamilbk deleted the fix/swift-last-dismissed branch February 14, 2025 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants