diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index 3d2601a87..858a60cc0 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en.lproj/Localizable.strings @@ -632,6 +632,7 @@ "screen_bug_report_error_description_too_short" = "The description is too short, please provide more details about what happened. Thanks!"; "screen_bug_report_include_crash_logs" = "Send crash logs"; "screen_bug_report_include_logs" = "Allow logs"; +"screen_bug_report_include_logs_error" = "Your logs are excessively large so cannot be included in this report, please send them to us another way."; "screen_bug_report_include_screenshot" = "Send screenshot"; "screen_bug_report_logs_description" = "Logs will be included with your message to make sure that everything is working properly. To send your message without logs, turn off this setting."; "screen_bug_report_view_logs" = "View logs"; diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index c255ac60a..1b9080ae4 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/ElementX/Sources/Generated/Strings.swift @@ -1224,6 +1224,8 @@ internal enum L10n { internal static var screenBugReportIncludeCrashLogs: String { return L10n.tr("Localizable", "screen_bug_report_include_crash_logs") } /// Allow logs internal static var screenBugReportIncludeLogs: String { return L10n.tr("Localizable", "screen_bug_report_include_logs") } + /// Your logs are excessively large so cannot be included in this report, please send them to us another way. + internal static var screenBugReportIncludeLogsError: String { return L10n.tr("Localizable", "screen_bug_report_include_logs_error") } /// Send screenshot internal static var screenBugReportIncludeScreenshot: String { return L10n.tr("Localizable", "screen_bug_report_include_screenshot") } /// Logs will be included with your message to make sure that everything is working properly. To send your message without logs, turn off this setting. diff --git a/ElementX/Sources/Screens/BugReportScreen/BugReportScreenModels.swift b/ElementX/Sources/Screens/BugReportScreen/BugReportScreenModels.swift index 9cf1ee9c6..a67d9cbe1 100644 --- a/ElementX/Sources/Screens/BugReportScreen/BugReportScreenModels.swift +++ b/ElementX/Sources/Screens/BugReportScreen/BugReportScreenModels.swift @@ -17,6 +17,7 @@ enum BugReportScreenViewModelAction { } struct BugReportScreenViewState: BindableState { + let canSendLogFiles: Bool var screenshot: UIImage? var bindings: BugReportScreenViewStateBindings let isModallyPresented: Bool diff --git a/ElementX/Sources/Screens/BugReportScreen/BugReportScreenViewModel.swift b/ElementX/Sources/Screens/BugReportScreen/BugReportScreenViewModel.swift index 2264d52e6..621040038 100644 --- a/ElementX/Sources/Screens/BugReportScreen/BugReportScreenViewModel.swift +++ b/ElementX/Sources/Screens/BugReportScreen/BugReportScreenViewModel.swift @@ -14,6 +14,8 @@ class BugReportScreenViewModel: BugReportScreenViewModelType, BugReportScreenVie private let bugReportService: BugReportServiceProtocol private let clientProxy: ClientProxyProtocol? + private let logFiles = Tracing.logFiles + private let actionsSubject: PassthroughSubject = .init() // periphery:ignore - when set to nil this is automatically cancelled @CancellableTask private var uploadTask: Task? @@ -29,8 +31,10 @@ class BugReportScreenViewModel: BugReportScreenViewModelType, BugReportScreenVie self.bugReportService = bugReportService self.clientProxy = clientProxy - let bindings = BugReportScreenViewStateBindings(reportText: "", sendingLogsEnabled: true, canContact: false) - super.init(initialViewState: BugReportScreenViewState(screenshot: screenshot, + let canSendLogFiles = Self.validate(logFiles) + let bindings = BugReportScreenViewStateBindings(reportText: "", sendingLogsEnabled: canSendLogFiles, canContact: false) + super.init(initialViewState: BugReportScreenViewState(canSendLogFiles: canSendLogFiles, + screenshot: screenshot, bindings: bindings, isModallyPresented: isModallyPresented)) } @@ -55,6 +59,23 @@ class BugReportScreenViewModel: BugReportScreenViewModelType, BugReportScreenVie } // MARK: Private + + /// Kind of a hack - when a log file balloons in size (e.g. due to a tight loop) the app is terminated by iOS for using + /// too much memory. This is because we compare the file size against the upload limit **after** compressing it, + /// but in order to compress it we load the whole file into memory. + /// + /// We could fix that, but then the problematic log file would be silently dropped and in reality it is much more valuable + /// to have the user contact us to say there's an issue with their logs so we can actually fix whatever is generating the + /// excessively large log files. + private static func validate(_ logFiles: [URL]) -> Bool { + for fileURL in logFiles { + if let size = try? FileManager.default.sizeForItem(at: fileURL), + size > Double(1024 * 1024 * 1024) { // Consider anything over 1GB to be excessive. + return false + } + } + return true + } private func submitBugReport() async { let progressSubject = CurrentValueSubject(0.0) @@ -81,7 +102,7 @@ class BugReportScreenViewModel: BugReportScreenViewModelType, BugReportScreenVie ed25519: ed25519, curve25519: curve25519, text: context.reportText, - includeLogs: context.sendingLogsEnabled, + logFiles: context.sendingLogsEnabled ? logFiles : nil, canContact: context.canContact, githubLabels: [], files: files) diff --git a/ElementX/Sources/Screens/BugReportScreen/View/BugReportScreen.swift b/ElementX/Sources/Screens/BugReportScreen/View/BugReportScreen.swift index d3926fc96..1419d534f 100644 --- a/ElementX/Sources/Screens/BugReportScreen/View/BugReportScreen.swift +++ b/ElementX/Sources/Screens/BugReportScreen/View/BugReportScreen.swift @@ -14,6 +14,7 @@ struct BugReportScreen: View { @Bindable var context: BugReportScreenViewModel.Context + var canSendLogFiles: Bool { context.viewState.canSendLogFiles } var photosPickerTitle: String { context.viewState.screenshot == nil ? L10n.screenBugReportAttachScreenshot : L10n.screenBugReportEditScreenshot } var body: some View { @@ -56,15 +57,23 @@ struct BugReportScreen: View { private var sendLogsSection: some View { Section { - ListRow(label: .plain(title: L10n.screenBugReportIncludeLogs), - kind: .toggle($context.sendingLogsEnabled)) - .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.sendLogs) + if canSendLogFiles { + ListRow(label: .plain(title: L10n.screenBugReportIncludeLogs), + kind: .toggle($context.sendingLogsEnabled)) + .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.sendLogs) + } ListRow(label: .plain(title: L10n.screenBugReportViewLogs), kind: .navigationLink { context.send(viewAction: .viewLogs) }) .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.sendLogs) } footer: { - Text(L10n.screenBugReportLogsDescription) - .compoundListSectionFooter() + if canSendLogFiles { + Text(L10n.screenBugReportLogsDescription) + .compoundListSectionFooter() + } else { + Label(L10n.screenBugReportIncludeLogsError, icon: \.errorSolid, iconSize: .xSmall, relativeTo: .compound.bodySM) + .foregroundStyle(.compound.textCriticalPrimary) + .compoundListSectionFooter() + } } } diff --git a/ElementX/Sources/Services/BugReport/BugReportService.swift b/ElementX/Sources/Services/BugReport/BugReportService.swift index e825eac8d..5a1823621 100644 --- a/ElementX/Sources/Services/BugReport/BugReportService.swift +++ b/ElementX/Sources/Services/BugReport/BugReportService.swift @@ -80,8 +80,8 @@ class BugReportService: NSObject, BugReportServiceProtocol { params.append(MultipartFormData(key: "label", type: .text(value: label))) } - if bugReport.includeLogs { - let logAttachments = await zipFiles(Tracing.logFiles) + if let logFiles = bugReport.logFiles { + let logAttachments = await zipFiles(logFiles) for url in logAttachments.files { params.append(MultipartFormData(key: "compressed-log", type: .file(url: url))) } diff --git a/ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift b/ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift index 87e3a9124..b46c4c435 100644 --- a/ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift +++ b/ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift @@ -15,7 +15,7 @@ struct BugReport: Equatable { let ed25519: String? let curve25519: String? let text: String - let includeLogs: Bool + let logFiles: [URL]? let canContact: Bool var githubLabels: [String] let files: [URL] diff --git a/Enterprise b/Enterprise index 5a3dd7063..a3ede7e4d 160000 --- a/Enterprise +++ b/Enterprise @@ -1 +1 @@ -Subproject commit 5a3dd7063f6bc33db9463f9a886d779dd8ff6a1c +Subproject commit a3ede7e4de5925d1ab4847b65bb1577b1429f12c diff --git a/UnitTests/Sources/BugReportScreenViewModelTests.swift b/UnitTests/Sources/BugReportScreenViewModelTests.swift index f1e157923..fd84ca394 100644 --- a/UnitTests/Sources/BugReportScreenViewModelTests.swift +++ b/UnitTests/Sources/BugReportScreenViewModelTests.swift @@ -79,14 +79,14 @@ class BugReportScreenViewModelTests: XCTestCase { context.send(viewAction: .submit) try await deferred.fulfill() - + XCTAssert(mockService.submitBugReportProgressListenerCallsCount == 1) XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.userID, "@mock.client.com") XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.deviceID, "ABCDEFGH") XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.curve25519, "THECURVEKEYKEY") XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.ed25519, "THEEDKEYKEY") XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.text, "This will succeed") - XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.includeLogs, true) + XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.logFiles?.isEmpty, false) XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.canContact, false) XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.githubLabels, []) XCTAssertEqual(mockService.submitBugReportProgressListenerReceivedArguments?.bugReport.files, []) diff --git a/UnitTests/Sources/BugReportServiceTests.swift b/UnitTests/Sources/BugReportServiceTests.swift index 3737448a3..0b4ee9212 100644 --- a/UnitTests/Sources/BugReportServiceTests.swift +++ b/UnitTests/Sources/BugReportServiceTests.swift @@ -31,7 +31,7 @@ class BugReportServiceTests: XCTestCase { ed25519: nil, curve25519: nil, text: "i cannot send message", - includeLogs: true, + logFiles: [URL(filePath: "/logs/1.log"), URL(filePath: "/logs/2.log")], canContact: false, githubLabels: [], files: []) @@ -76,7 +76,7 @@ class BugReportServiceTests: XCTestCase { ed25519: nil, curve25519: nil, text: "i cannot send message", - includeLogs: true, + logFiles: Tracing.logFiles, canContact: false, githubLabels: [], files: [])