Merge pull request #34 from am7590/more-stability-fixes

Improve iOS polling reliability, error handling and logging
This commit is contained in:
Philipp C. Heckel 2026-04-14 21:20:25 -04:00 committed by GitHub
commit f4e5521b6e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 156 additions and 62 deletions

View file

@ -52,20 +52,43 @@ class AppDelegate: UIResponder, UIApplicationDelegate, ObservableObject {
// Poll and show new messages as notifications
let store = Store.shared
let subscriptionManager = SubscriptionManager(store: store)
store.getSubscriptions()?.forEach { subscription in
let subscriptions = store.getSubscriptions() ?? []
guard !subscriptions.isEmpty else {
completionHandler(.noData)
return
}
let group = DispatchGroup()
let resultQueue = DispatchQueue(label: "io.heckel.ntfy.background-poll-result")
var didReceiveNewData = false
subscriptions.forEach { subscription in
group.enter()
guard let baseUrl = subscription.baseUrl else {
Log.w(tag, "Skipping background poll notification for subscription with missing baseUrl")
group.leave()
return
}
subscriptionManager.poll(subscription) { messages in
messages.forEach { message in
self.showNotification(subscription, message)
if !messages.isEmpty {
resultQueue.sync {
didReceiveNewData = true
}
}
messages.forEach { message in
self.showNotification(baseUrl: baseUrl, message)
}
group.leave()
}
}
completionHandler(.newData)
group.notify(queue: .main) {
completionHandler(didReceiveNewData ? .newData : .noData)
}
}
func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
let token = deviceToken.map { data in String(format: "%02.2hhx", data) }.joined()
Messaging.messaging().apnsToken = deviceToken
Log.d(tag, "Registered for remote notifications. Passing APNs token to Firebase: \(token)")
Log.d(tag, "Registered for remote notifications. Passing APNs token \(token.prefix(12))... to Firebase")
}
func application(_ application: UIApplication, didFailToRegisterForRemoteNotificationsWithError error: Error) {
@ -76,8 +99,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate, ObservableObject {
/// local notification look exactly like the remote one (same userInfo), so that when we tap it, the userNotificationCenter(didReceive) function
/// has the same information available.
private func showNotification(_ subscription: Subscription, _ message: Message) {
guard let baseUrl = subscription.baseUrl else {
Log.w(tag, "Skipping notification for subscription with missing baseUrl")
return
}
showNotification(baseUrl: baseUrl, message)
}
private func showNotification(baseUrl: String, _ message: Message) {
let content = UNMutableNotificationContent()
content.modify(message: message, baseUrl: subscription.baseUrl ?? "?")
content.modify(message: message, baseUrl: baseUrl)
let request = UNNotificationRequest(identifier: message.id, content: content, trigger: nil /* now */)
UNUserNotificationCenter.current().add(request) { (error) in
@ -135,18 +166,34 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
extension AppDelegate: MessagingDelegate {
func messaging(_ messaging: Messaging, didReceiveRegistrationToken fcmToken: String?) {
Log.d(tag, "Firebase token received: \(String(describing: fcmToken))")
if let fcmToken = fcmToken, !fcmToken.isEmpty {
Log.d(tag, "Firebase token received: \(fcmToken.prefix(12))...")
} else {
Log.w(tag, "Firebase token missing")
}
// Subscribe to ~poll topic
Messaging.messaging().subscribe(toTopic: pollTopic)
Messaging.messaging().subscribe(toTopic: pollTopic) { error in
if let error {
Log.e(self.tag, "Firebase subscribe failed for \(self.pollTopic)", error)
} else {
Log.d(self.tag, "Firebase subscribe succeeded for \(self.pollTopic)")
}
}
// Re-subscribe to Firebase for all topics
let store = Store.shared
let subscriptionManager = SubscriptionManager(store: store)
store.getSubscriptions()?.forEach{ subscription in
if let baseUrl = subscription.baseUrl, let topic = subscription.topic {
let firebaseTopicName = firebaseTopic(baseUrl: baseUrl, topic: topic)
Log.d(tag, "Re-subscribing to topic \(baseUrl)/\(topic)")
Messaging.messaging().subscribe(toTopic: firebaseTopic(baseUrl: baseUrl, topic: topic))
Messaging.messaging().subscribe(toTopic: firebaseTopicName) { error in
if let error {
Log.e(self.tag, "Firebase subscribe failed for \(firebaseTopicName)", error)
} else {
Log.d(self.tag, "Firebase subscribe succeeded for \(firebaseTopicName)")
}
}
}
}
}

View file

@ -101,11 +101,11 @@ struct Message: Decodable {
let time = userInfo["time"] as? String,
let event = userInfo["event"] as? String,
let topic = userInfo["topic"] as? String,
let timeInt = Int64(time),
let message = userInfo["message"] as? String else {
let timeInt = Int64(time) else {
Log.d(Store.tag, "Unknown or irrelevant message", userInfo)
return nil
}
let message = userInfo["message"] as? String
let title = userInfo["title"] as? String
let priority = Int16(userInfo["priority"] as? String ?? "3") ?? 3
let tags = (userInfo["tags"] as? String ?? "").components(separatedBy: ",")

View file

@ -115,24 +115,34 @@ class Store: ObservableObject {
// MARK: Notifications
func save(notificationFromMessage message: Message, withSubscription subscription: Subscription) {
do {
let notification = Notification(context: context)
notification.id = message.id
notification.time = message.time
notification.message = message.message ?? ""
notification.title = message.title ?? ""
notification.priority = (message.priority != nil && message.priority != 0) ? message.priority! : 3
notification.tags = message.tags?.joined(separator: ",") ?? ""
notification.actions = Actions.shared.encode(message.actions)
notification.click = message.click ?? ""
notification.subscription = subscription
subscription.addToNotifications(notification)
subscription.lastNotificationId = message.id
Log.d(Store.tag, "Storing notification with ID \(notification.id ?? "<unknown>")")
try context.save()
} catch let error {
Log.w(Store.tag, "Cannot store notification (fromMessage)", error)
rollbackAndRefresh()
save(notificationsFromMessages: [message], withSubscription: subscription)
}
func save(notificationsFromMessages messages: [Message], withSubscription subscription: Subscription) {
guard !messages.isEmpty else { return }
context.performAndWait {
do {
for message in messages {
let notification = Notification(context: context)
notification.id = message.id
notification.time = message.time
notification.message = message.message ?? ""
notification.title = message.title ?? ""
notification.priority = (message.priority != nil && message.priority != 0) ? message.priority! : 3
notification.tags = message.tags?.joined(separator: ",") ?? ""
notification.actions = Actions.shared.encode(message.actions)
notification.click = message.click ?? ""
notification.subscription = subscription
subscription.addToNotifications(notification)
subscription.lastNotificationId = message.id
Log.d(Store.tag, "Storing notification with ID \(notification.id ?? "<unknown>")")
}
try context.save()
} catch let error {
Log.w(Store.tag, "Cannot store notifications (fromMessages)", error)
rollbackAndRefresh()
}
}
}

View file

@ -9,8 +9,15 @@ struct SubscriptionManager {
func subscribe(baseUrl: String, topic: String) {
let normalizedBaseUrl = normalizeBaseUrl(baseUrl)
let firebaseTopicName = firebaseTopic(baseUrl: normalizedBaseUrl, topic: topic)
Log.d(tag, "Subscribing to \(topicUrl(baseUrl: normalizedBaseUrl, topic: topic))")
Messaging.messaging().subscribe(toTopic: firebaseTopic(baseUrl: normalizedBaseUrl, topic: topic))
Messaging.messaging().subscribe(toTopic: firebaseTopicName) { error in
if let error {
Log.e(tag, "Firebase subscribe failed for \(firebaseTopicName)", error)
} else {
Log.d(tag, "Firebase subscribe succeeded for \(firebaseTopicName)")
}
}
let subscription = store.saveSubscription(baseUrl: normalizedBaseUrl, topic: topic)
poll(subscription)
}
@ -19,7 +26,14 @@ struct SubscriptionManager {
Log.d(tag, "Unsubscribing from \(subscription.urlString())")
DispatchQueue.main.async {
if let baseUrl = subscription.baseUrl, let topic = subscription.topic {
Messaging.messaging().unsubscribe(fromTopic: firebaseTopic(baseUrl: baseUrl, topic: topic))
let firebaseTopicName = firebaseTopic(baseUrl: baseUrl, topic: topic)
Messaging.messaging().unsubscribe(fromTopic: firebaseTopicName) { error in
if let error {
Log.e(tag, "Firebase unsubscribe failed for \(firebaseTopicName)", error)
} else {
Log.d(tag, "Firebase unsubscribe succeeded for \(firebaseTopicName)")
}
}
}
store.delete(subscription: subscription)
}
@ -47,11 +61,7 @@ struct SubscriptionManager {
}
Log.d(tag, "Polling success, \(messages.count) new message(s)", messages)
if !messages.isEmpty {
DispatchQueue.main.sync {
for message in messages {
store.save(notificationFromMessage: message, withSubscription: subscription)
}
}
store.save(notificationsFromMessages: messages, withSubscription: subscription)
}
completionHandler(messages)
}

View file

@ -8,7 +8,7 @@ class ApiService {
func poll(subscription: Subscription, user: BasicUser?, completionHandler: @escaping ([Message]?, Error?) -> Void) {
guard let url = URL(string: subscription.urlString()) else {
// FIXME
completionHandler(nil, URLError(.badURL))
return
}
let since = subscription.lastNotificationId ?? "all"
@ -19,7 +19,10 @@ class ApiService {
}
func poll(subscription: Subscription, messageId: String, user: BasicUser?, completionHandler: @escaping (Message?, Error?) -> Void) {
let url = URL(string: "\(subscription.urlString())/json?poll=1&id=\(messageId)")!
guard let url = URL(string: "\(subscription.urlString())/json?poll=1&id=\(messageId)") else {
completionHandler(nil, URLError(.badURL))
return
}
Log.d(tag, "Polling single message from \(url) with user \(user?.username ?? "anonymous")")
let request = newRequest(url: url, user: user)
@ -28,8 +31,16 @@ class ApiService {
completionHandler(nil, error)
return
}
guard let httpResponse = response as? HTTPURLResponse, (200..<300).contains(httpResponse.statusCode) else {
completionHandler(nil, URLError(.badServerResponse))
return
}
guard let data = data else {
completionHandler(nil, URLError(.badServerResponse))
return
}
do {
let message = try JSONDecoder().decode(Message.self, from: data!)
let message = try JSONDecoder().decode(Message.self, from: data)
completionHandler(message, nil)
} catch {
completionHandler(nil, error)
@ -98,19 +109,33 @@ class ApiService {
}
private func fetchJsonData<T: Decodable>(urlString: String, user: BasicUser?, completionHandler: @escaping ([T]?, Error?) -> ()) {
guard let url = URL(string: urlString) else { return }
guard let url = URL(string: urlString) else {
completionHandler(nil, URLError(.badURL))
return
}
let request = newRequest(url: url, user: user)
newSession(timeout: 30).dataTask(with: request) { (data, response, error) in
if let error = error {
if let error {
Log.e(self.tag, "Error fetching data", error)
completionHandler(nil, error)
return
}
guard let httpResponse = response as? HTTPURLResponse, (200..<300).contains(httpResponse.statusCode) else {
completionHandler(nil, URLError(.badServerResponse))
return
}
guard let data = data else {
completionHandler(nil, URLError(.badServerResponse))
return
}
do {
let lines = String(decoding: data!, as: UTF8.self).split(whereSeparator: \.isNewline)
let lines = String(decoding: data, as: UTF8.self).split(whereSeparator: \.isNewline)
var notifications: [T] = []
for jsonLine in lines {
notifications.append(try JSONDecoder().decode(T.self, from: jsonLine.data(using: .utf8)!))
guard let jsonData = jsonLine.data(using: .utf8) else {
throw URLError(.cannotDecodeContentData)
}
notifications.append(try JSONDecoder().decode(T.self, from: jsonData))
}
completionHandler(notifications, nil)
} catch {

View file

@ -19,18 +19,14 @@ struct SubscriptionListView: View {
if #available(iOS 15.0, *) {
subscriptionList
.refreshable {
subscriptionsModel.subscriptions.forEach { subscription in
subscriptionManager.poll(subscription)
}
pollSubscriptions()
}
} else {
subscriptionList
.toolbar {
ToolbarItem(placement: .navigationBarLeading) {
Button {
subscriptionsModel.subscriptions.forEach { subscription in
subscriptionManager.poll(subscription)
}
pollSubscriptions()
} label: {
Image(systemName: "arrow.clockwise")
}
@ -81,6 +77,16 @@ struct SubscriptionListView: View {
.sheet(isPresented: $showingAddDialog) {
SubscriptionAddView(isShowing: $showingAddDialog)
}
.onAppear {
// Ensures subscription count stays up to date, so a pull to refresh isn't required
pollSubscriptions()
}
}
private func pollSubscriptions() {
subscriptionsModel.subscriptions.forEach { subscription in
subscriptionManager.poll(subscription)
}
}
}

View file

@ -66,35 +66,31 @@ class NotificationService: UNNotificationServiceExtension {
}
private func handlePollRequest(_ request: UNNotificationRequest, _ content: UNMutableNotificationContent, _ pollRequest: Message, _ contentHandler: @escaping (UNNotificationContent) -> Void) {
let subscription = store?.getSubscriptions()?.first { $0.urlHash() == pollRequest.topic }
let subscription = store?.getSubscriptions()?.first { subscription in
// Poll requests usually target the hashed topic URL, but tolerate raw topic payloads too
subscription.urlHash() == pollRequest.topic || subscription.topic == pollRequest.topic
}
let baseUrl = subscription?.baseUrl
let pollId = pollRequest.pollId ?? pollRequest.id
guard
let subscription = subscription,
let pollId = pollRequest.pollId,
let baseUrl = baseUrl
else {
Log.w(tag, "Cannot find subscription", pollRequest)
Log.w(tag, "Cannot find subscription for poll request topic=\(pollRequest.topic), pollId=\(pollRequest.pollId ?? "<nil>")")
contentHandler(request.content)
return
}
// Poll original server
let user = store?.getUser(baseUrl: baseUrl)?.toBasicUser()
let semaphore = DispatchSemaphore(value: 0)
// The extension only needs contentHandler to be called from the async callback
ApiService.shared.poll(subscription: subscription, messageId: pollId, user: user) { message, error in
guard let message = message else {
Log.w(self.tag, "Error fetching message", error)
Log.w(self.tag, "Error fetching poll request message topic=\(pollRequest.topic), pollId=\(pollId), subscription=\(subscription.urlString())", error)
contentHandler(request.content)
return
}
self.handleMessage(request, content, baseUrl, message, contentHandler)
semaphore.signal()
}
// Note: If notifications only show up as "New message", it may be because the "return" statement
// happens before the contentHandler() is called. We add this semaphore here to synchronize the threads.
// I don't know if this is necessary, but it feels like the right thing to do.
_ = semaphore.wait(timeout: DispatchTime.now() + 25) // 30 seconds is the max for the entire extension
}
}