From 3cf94ad8813d8538f99ba583e6cccfc118df7e68 Mon Sep 17 00:00:00 2001 From: Maximilian Bauer Date: Wed, 2 Jun 2021 21:17:42 +0200 Subject: [PATCH] EventLogger: display more errors to user --- .../Api/Ampache/AmpacheLibrarySyncer.swift | 6 +-- Amperfy/Api/EventLogger.swift | 54 ++++++++++++++----- Amperfy/Common/AppDelegate.swift | 2 +- Amperfy/Download/DownloadManager.swift | 22 ++++++-- Amperfy/Download/UrlDownloader.swift | 2 +- Amperfy/Player/BackendAudioPlayer.swift | 2 +- .../Screens/ViewController/SupportVC.swift | 2 +- Amperfy/Storage/EntityWrappers/LogEntry.swift | 6 +-- 8 files changed, 69 insertions(+), 27 deletions(-) diff --git a/Amperfy/Api/Ampache/AmpacheLibrarySyncer.swift b/Amperfy/Api/Ampache/AmpacheLibrarySyncer.swift index 14475709..09f74395 100644 --- a/Amperfy/Api/Ampache/AmpacheLibrarySyncer.swift +++ b/Amperfy/Api/Ampache/AmpacheLibrarySyncer.swift @@ -102,15 +102,15 @@ class AmpacheLibrarySyncer: LibrarySyncer { } func syncMusicFolders(libraryStorage: LibraryStorage) { - ampacheXmlServerApi.eventLogger.info(message: "GetMusicFolders API function is not support by Ampache") + ampacheXmlServerApi.eventLogger.error(topic: "Internal Error", statusCode: .internalError, message: "GetMusicFolders API function is not support by Ampache") } func syncIndexes(musicFolder: MusicFolder, libraryStorage: LibraryStorage) { - ampacheXmlServerApi.eventLogger.info(message: "GetIndexes API function is not support by Ampache") + ampacheXmlServerApi.eventLogger.error(topic: "Internal Error", statusCode: .internalError, message: "GetIndexes API function is not support by Ampache") } func sync(directory: Directory, libraryStorage: LibraryStorage) { - ampacheXmlServerApi.eventLogger.info(message: "GetMusicDirectory API function is not support by Ampache") + ampacheXmlServerApi.eventLogger.error(topic: "Internal Error", statusCode: .internalError, message: "GetMusicDirectory API function is not support by Ampache") } func syncDownPlaylistsWithoutSongs(libraryStorage: LibraryStorage) { diff --git a/Amperfy/Api/EventLogger.swift b/Amperfy/Api/EventLogger.swift index be362eb1..0ffee657 100644 --- a/Amperfy/Api/EventLogger.swift +++ b/Amperfy/Api/EventLogger.swift @@ -7,6 +7,13 @@ protocol AlertDisplayable { func display(alert: UIAlertController) // Must be called from main thread } +enum AmperfyLogStatusCode: Int { + case downloadError = 1 + case playerError = 2 + case emailError = 3 + case internalError = 4 +} + class EventLogger { static private let errorReportOneDaySilentTimeInSec = 60*60*24 @@ -20,30 +27,47 @@ class EventLogger { self.persistentContainer = persistentContainer } - func info(message: String) { - os_log("Info: %s", log: log, type: .info, message) - displayAlert(message: message) + func info(topic: String, statusCode: AmperfyLogStatusCode, message: String) { + report(topic: topic, statusCode: statusCode, message: message, logType: .info) + } + + func error(topic: String, statusCode: AmperfyLogStatusCode, message: String) { + report(topic: topic, statusCode: statusCode, message: message, logType: .error) + } + + private func report(topic: String, statusCode: AmperfyLogStatusCode, message: String, logType: LogEntryType) { + persistentContainer.performBackgroundTask{ context in + let library = LibraryStorage(context: context) + let logEntries = library.getLogEntries() + let sameStatusCodeEntries = logEntries.lazy.filter{ $0.statusCode == statusCode.rawValue && $0.type == logType } + if let sameEntry = sameStatusCodeEntries.first, sameEntry.creationDate.compare(Date() - Double(sameEntry.suppressionTimeInterval)) == .orderedDescending { + return + } + self.displayAlert(topic: topic, statusCode: statusCode, message: message, logType: logType) + } } - private func displayAlert(message: String) { + private func displayAlert(topic: String, statusCode: AmperfyLogStatusCode, message: String, logType: LogEntryType) { DispatchQueue.main.async { var alertMessage = "" alertMessage += "\(message)" - let alert = UIAlertController(title: "Info", message: alertMessage, preferredStyle: .alert) + let alert = UIAlertController(title: topic, message: alertMessage, preferredStyle: .alert) + alert.addAction(UIAlertAction(title: "Suppress for \(Self.errorReportOneDaySilentTimeInSec.asDayString)", style: .destructive , handler: { _ in + self.saveMessagePersistent(message: message, statusCode: statusCode, logType: logType, suppressionTimeInterval: Self.errorReportOneDaySilentTimeInSec) + })) alert.addAction(UIAlertAction(title: "OK", style: .default , handler: { _ in - self.saveInfoPersistent(message: message) + self.saveMessagePersistent(message: message, statusCode: statusCode, logType: logType) })) self.alertDisplayer.display(alert: alert) } } func report(error: ResponseError) { - os_log("API-ERROR StatusCode: %d, Message: %s", log: log, type: .error, error.statusCode, error.message) persistentContainer.performBackgroundTask{ context in let library = LibraryStorage(context: context) let logEntries = library.getLogEntries() - let sameStatusCodeEntries = logEntries.lazy.filter{ $0.statusCode == error.statusCode } + let sameStatusCodeEntries = logEntries.lazy.filter{ $0.statusCode == error.statusCode && $0.type == .apiError } if let sameEntry = sameStatusCodeEntries.first, sameEntry.creationDate.compare(Date() - Double(sameEntry.suppressionTimeInterval)) == .orderedDescending { return } @@ -60,22 +84,22 @@ class EventLogger { alertMessage += "\nYou can find the event log at:" alertMessage += "\nSettings -> Support -> Event Log" - let alert = UIAlertController(title: "API Error Occured", message: alertMessage, preferredStyle: .alert) + let alert = UIAlertController(title: "API Error", message: alertMessage, preferredStyle: .alert) alert.addAction(UIAlertAction(title: "Suppress for \(Self.errorReportOneDaySilentTimeInSec.asDayString)", style: .destructive , handler: { _ in self.saveErrorPersistent(error: error, suppressionTimeInterval: Self.errorReportOneDaySilentTimeInSec) })) alert.addAction(UIAlertAction(title: "OK", style: .default , handler: { _ in - self.saveErrorPersistent(error: error, suppressionTimeInterval: 0) + self.saveErrorPersistent(error: error) })) self.alertDisplayer.display(alert: alert) } } - private func saveErrorPersistent(error: ResponseError, suppressionTimeInterval: Int) { + private func saveErrorPersistent(error: ResponseError, suppressionTimeInterval: Int = 0) { persistentContainer.performBackgroundTask{ context in let library = LibraryStorage(context: context) let errorLog = library.createLogEntry() - errorLog.type = .error + errorLog.type = .apiError errorLog.statusCode = error.statusCode errorLog.message = error.message errorLog.suppressionTimeInterval = suppressionTimeInterval @@ -83,12 +107,14 @@ class EventLogger { } } - private func saveInfoPersistent(message: String) { + private func saveMessagePersistent(message: String, statusCode: AmperfyLogStatusCode, logType: LogEntryType, suppressionTimeInterval: Int = 0) { persistentContainer.performBackgroundTask{ context in let library = LibraryStorage(context: context) let errorLog = library.createLogEntry() - errorLog.type = .info + errorLog.type = logType + errorLog.statusCode = statusCode.rawValue errorLog.message = message + errorLog.suppressionTimeInterval = suppressionTimeInterval library.saveContext() } } diff --git a/Amperfy/Common/AppDelegate.swift b/Amperfy/Common/AppDelegate.swift index 930fff4d..d7fc8acc 100644 --- a/Amperfy/Common/AppDelegate.swift +++ b/Amperfy/Common/AppDelegate.swift @@ -36,7 +36,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { let requestManager = RequestManager() let dlDelegate = DownloadDelegate(backendApi: backendApi) let urlDownloader = UrlDownloader(requestManager: requestManager) - let dlManager = DownloadManager(storage: storage, requestManager: requestManager, urlDownloader: urlDownloader, downloadDelegate: dlDelegate) + let dlManager = DownloadManager(storage: storage, requestManager: requestManager, urlDownloader: urlDownloader, downloadDelegate: dlDelegate, eventLogger: eventLogger) urlDownloader.urlDownloadNotifier = dlManager return dlManager }() diff --git a/Amperfy/Download/DownloadManager.swift b/Amperfy/Download/DownloadManager.swift index 55bb7c5a..541ce182 100644 --- a/Amperfy/Download/DownloadManager.swift +++ b/Amperfy/Download/DownloadManager.swift @@ -7,7 +7,19 @@ enum DownloadError: Error { case noConnectivity case alreadyDownloaded case fetchFailed + case emptyFile case apiErrorResponse + + var description : String { + switch self { + case .urlInvalid: return "Invalid URL" + case .noConnectivity: return "No Connectivity" + case .alreadyDownloaded: return "Already Downloaded" + case .fetchFailed: return "Fetch Failed" + case .emptyFile: return "File is empty" + case .apiErrorResponse: return "API Error" + } + } } protocol SongDownloadable { @@ -37,6 +49,7 @@ class DownloadManager: SongDownloadable { private let requestManager: RequestManager private let urlDownloader: UrlDownloader private let downloadDelegate: DownloadManagerDelegate + private let eventLogger: EventLogger private let downloadSlotCounter = DownloadSlotCounter(maximumActiveDownloads: 4) @@ -49,11 +62,12 @@ class DownloadManager: SongDownloadable { return requestManager.requestQueues } - init(storage: PersistentStorage, requestManager: RequestManager, urlDownloader: UrlDownloader, downloadDelegate: DownloadManagerDelegate) { + init(storage: PersistentStorage, requestManager: RequestManager, urlDownloader: UrlDownloader, downloadDelegate: DownloadManagerDelegate, eventLogger: EventLogger) { self.storage = storage self.requestManager = requestManager self.urlDownloader = urlDownloader self.downloadDelegate = downloadDelegate + self.eventLogger = eventLogger } func download(song: Song, notifier: SongDownloadNotifiable? = nil, priority: Priority = .low) { @@ -147,12 +161,14 @@ class DownloadManager: SongDownloadable { os_log("Fetching %s SUCCESS (%{iec-bytes}d)", log: self.log, type: .info, request.title, request.download?.resumeData?.count ?? 0) downloadDelegate.completedDownload(request: request, context: context) } catch let fetchError as DownloadError { - os_log("Fetching %s FAILED", log: self.log, type: .info, request.title) downloadError = fetchError } catch { - os_log("Fetching %s FAILED", log: self.log, type: .info, request.title) downloadError = DownloadError.fetchFailed } + if let error = downloadError, error != .apiErrorResponse { + os_log("Fetching %s FAILED: %s", log: self.log, type: .info, request.title, error.description) + eventLogger.error(topic: "Download Error", statusCode: .downloadError, message: "Error \"\(error.description)\" occured while downloading song \"\(request.title)\".") + } // remove data from request to free memory request.download?.resumeData = nil self.requestManager.informDownloadCompleted(request: request) diff --git a/Amperfy/Download/UrlDownloader.swift b/Amperfy/Download/UrlDownloader.swift index dc77bfac..6188ff46 100644 --- a/Amperfy/Download/UrlDownloader.swift +++ b/Amperfy/Download/UrlDownloader.swift @@ -46,7 +46,7 @@ class UrlDownloader: NSObject, URLSessionDownloadDelegate { if data.count > 0 { download.resumeData = data } else { - download.error = .fetchFailed + download.error = .emptyFile } } catch let error { diff --git a/Amperfy/Player/BackendAudioPlayer.swift b/Amperfy/Player/BackendAudioPlayer.swift index ef8ede3e..6a390437 100644 --- a/Amperfy/Player/BackendAudioPlayer.swift +++ b/Amperfy/Player/BackendAudioPlayer.swift @@ -116,7 +116,7 @@ class BackendAudioPlayer: SongDownloadNotifiable { if !song.isPlayableOniOS, let contentType = song.contentType { player.pause() player.replaceCurrentItem(with: nil) - eventLogger.info(message: "Content type \"\(contentType)\" of song \"\(song.displayString)\" is not playable via Amperfy.") + eventLogger.info(topic: "Player Info", statusCode: .playerError, message: "Content type \"\(contentType)\" of song \"\(song.displayString)\" is not playable via Amperfy.") } else { if song.isCached { insertCachedSong(playlistItem: playlistItem) diff --git a/Amperfy/Screens/ViewController/SupportVC.swift b/Amperfy/Screens/ViewController/SupportVC.swift index 50401c15..97f74357 100644 --- a/Amperfy/Screens/ViewController/SupportVC.swift +++ b/Amperfy/Screens/ViewController/SupportVC.swift @@ -36,7 +36,7 @@ class SupportVC: UITableViewController, MFMailComposeViewControllerDelegate { mailComposer.mailComposeDelegate = self self.present(mailComposer, animated: true, completion: nil) } else { - appDelegate.eventLogger.info(message: "Email is not configured in settings app or Amperfy is not able to send an email") + appDelegate.eventLogger.info(topic: "Email Info", statusCode: .emailError, message: "Email is not configured in settings app or Amperfy is not able to send an email") } } diff --git a/Amperfy/Storage/EntityWrappers/LogEntry.swift b/Amperfy/Storage/EntityWrappers/LogEntry.swift index 107280b4..c065febf 100644 --- a/Amperfy/Storage/EntityWrappers/LogEntry.swift +++ b/Amperfy/Storage/EntityWrappers/LogEntry.swift @@ -2,15 +2,15 @@ import Foundation import CoreData enum LogEntryType: Int16 { - case error = 0 - case warning = 1 + case apiError = 0 + case error = 1 case info = 2 case debug = 3 var description : String { switch self { + case .apiError: return "API Error" case .error: return "Error" - case .warning: return "Warning" case .info: return "Info" case .debug: return "Debug" }