diff --git a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt index deddbd4faa..0af13dcdda 100644 --- a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt +++ b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt @@ -16,15 +16,10 @@ package io.element.android.features.rageshake.api.reporter -import io.element.android.features.rageshake.api.reporter.BugReporterListener -import io.element.android.features.rageshake.api.reporter.ReportType -import kotlinx.coroutines.CoroutineScope - interface BugReporter { /** * Send a bug report. * - * @param coroutineScope The coroutine scope * @param reportType The report type (bug, suggestion, feedback) * @param withDevicesLogs true to include the device log * @param withCrashLogs true to include the crash logs @@ -36,8 +31,7 @@ interface BugReporter { * @param customFields fields which will be sent with the report * @param listener the listener */ - fun sendBugReport( - coroutineScope: CoroutineScope, + suspend fun sendBugReport( reportType: ReportType, withDevicesLogs: Boolean, withCrashLogs: Boolean, diff --git a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenter.kt b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenter.kt index bce4e96709..5acb2bd4b1 100644 --- a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenter.kt +++ b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenter.kt @@ -132,7 +132,6 @@ class BugReportPresenter @Inject constructor( listener: BugReporterListener, ) = launch { bugReporter.sendBugReport( - coroutineScope = this, reportType = ReportType.BUG_REPORT, withDevicesLogs = formState.sendLogs, withCrashLogs = hasCrashLogs && formState.sendCrashLogs, diff --git a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt index 4e031703ab..d9e56a91bc 100755 --- a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt +++ b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt @@ -21,13 +21,13 @@ import android.os.Build import androidx.core.net.toFile import androidx.core.net.toUri import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.features.rageshake.api.crash.CrashDataStore import io.element.android.features.rageshake.api.reporter.BugReporter import io.element.android.features.rageshake.api.reporter.BugReporterListener import io.element.android.features.rageshake.api.reporter.ReportType +import io.element.android.features.rageshake.api.screenshot.ScreenshotHolder import io.element.android.features.rageshake.impl.R -import io.element.android.features.rageshake.api.crash.CrashDataStore import io.element.android.features.rageshake.impl.logs.VectorFileLogger -import io.element.android.features.rageshake.api.screenshot.ScreenshotHolder import io.element.android.libraries.androidutils.file.compressFile import io.element.android.libraries.androidutils.file.safeDelete import io.element.android.libraries.core.coroutine.CoroutineDispatchers @@ -35,9 +35,7 @@ import io.element.android.libraries.core.extensions.toOnOff import io.element.android.libraries.core.mimetype.MimeTypes import io.element.android.libraries.di.AppScope import io.element.android.libraries.di.ApplicationContext -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.first -import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import okhttp3.Call import okhttp3.MediaType.Companion.toMediaTypeOrNull @@ -116,7 +114,6 @@ class DefaultBugReporter @Inject constructor( /** * Send a bug report. * - * @param coroutineScope The coroutine scope * @param reportType The report type (bug, suggestion, feedback) * @param withDevicesLogs true to include the device log * @param withCrashLogs true to include the crash logs @@ -128,8 +125,7 @@ class DefaultBugReporter @Inject constructor( * @param customFields fields which will be sent with the report * @param listener the listener */ - override fun sendBugReport( - coroutineScope: CoroutineScope, + override suspend fun sendBugReport( reportType: ReportType, withDevicesLogs: Boolean, withCrashLogs: Boolean, @@ -144,282 +140,280 @@ class DefaultBugReporter @Inject constructor( // enumerate files to delete val mBugReportFiles: MutableList = ArrayList() - coroutineScope.launch { - var serverError: String? = null - var reportURL: String? = null - withContext(coroutineDispatchers.io) { - var bugDescription = theBugDescription - val crashCallStack = crashDataStore.crashInfo().first() + var serverError: String? = null + var reportURL: String? = null + withContext(coroutineDispatchers.io) { + var bugDescription = theBugDescription + val crashCallStack = crashDataStore.crashInfo().first() - if (crashCallStack.isNotEmpty() && withCrashLogs) { - bugDescription += "\n\n\n\n--------------------------------- crash call stack ---------------------------------\n" - bugDescription += crashCallStack - } + if (crashCallStack.isNotEmpty() && withCrashLogs) { + bugDescription += "\n\n\n\n--------------------------------- crash call stack ---------------------------------\n" + bugDescription += crashCallStack + } - val gzippedFiles = ArrayList() + val gzippedFiles = ArrayList() - val vectorFileLogger = VectorFileLogger.getFromTimber() - if (withDevicesLogs && vectorFileLogger != null) { - val files = vectorFileLogger.getLogFiles() - files.mapNotNullTo(gzippedFiles) { f -> - if (!mIsCancelled) { - compressFile(f) - } else { - null - } + val vectorFileLogger = VectorFileLogger.getFromTimber() + if (withDevicesLogs && vectorFileLogger != null) { + val files = vectorFileLogger.getLogFiles() + files.mapNotNullTo(gzippedFiles) { f -> + if (!mIsCancelled) { + compressFile(f) + } else { + null } } + } - if (!mIsCancelled && (withCrashLogs || withDevicesLogs)) { - val gzippedLogcat = saveLogCat(false) + if (!mIsCancelled && (withCrashLogs || withDevicesLogs)) { + val gzippedLogcat = saveLogCat(false) - if (null != gzippedLogcat) { - if (gzippedFiles.size == 0) { - gzippedFiles.add(gzippedLogcat) - } else { - gzippedFiles.add(0, gzippedLogcat) - } + if (null != gzippedLogcat) { + if (gzippedFiles.size == 0) { + gzippedFiles.add(gzippedLogcat) + } else { + gzippedFiles.add(0, gzippedLogcat) } } + } - /* - activeSessionHolder.getSafeActiveSession() - ?.takeIf { !mIsCancelled && withKeyRequestHistory } - ?.cryptoService() - ?.getGossipingEvents() - ?.let { GossipingEventsSerializer().serialize(it) } - ?.toByteArray() - ?.let { rawByteArray -> - File(context.cacheDir.absolutePath, KEY_REQUESTS_FILENAME) - .also { - it.outputStream() - .use { os -> os.write(rawByteArray) } - } - } - ?.let { compressFile(it) } - ?.let { gzippedFiles.add(it) } - */ - - var deviceId = "undefined" - var userId = "undefined" - var olmVersion = "undefined" - - /* - activeSessionHolder.getSafeActiveSession()?.let { session -> - userId = session.myUserId - deviceId = session.sessionParams.deviceId ?: "undefined" - olmVersion = session.cryptoService().getCryptoVersion(context, true) - } - */ - - if (!mIsCancelled) { - val text = when (reportType) { - ReportType.BUG_REPORT -> bugDescription - ReportType.SUGGESTION -> "[Suggestion] $bugDescription" - ReportType.SPACE_BETA_FEEDBACK -> "[spaces-feedback] $bugDescription" - ReportType.THREADS_BETA_FEEDBACK -> "[threads-feedback] $bugDescription" - ReportType.AUTO_UISI_SENDER, - ReportType.AUTO_UISI -> bugDescription + /* + activeSessionHolder.getSafeActiveSession() + ?.takeIf { !mIsCancelled && withKeyRequestHistory } + ?.cryptoService() + ?.getGossipingEvents() + ?.let { GossipingEventsSerializer().serialize(it) } + ?.toByteArray() + ?.let { rawByteArray -> + File(context.cacheDir.absolutePath, KEY_REQUESTS_FILENAME) + .also { + it.outputStream() + .use { os -> os.write(rawByteArray) } + } } + ?.let { compressFile(it) } + ?.let { gzippedFiles.add(it) } + */ + + var deviceId = "undefined" + var userId = "undefined" + var olmVersion = "undefined" + + /* + activeSessionHolder.getSafeActiveSession()?.let { session -> + userId = session.myUserId + deviceId = session.sessionParams.deviceId ?: "undefined" + olmVersion = session.cryptoService().getCryptoVersion(context, true) + } + */ + + if (!mIsCancelled) { + val text = when (reportType) { + ReportType.BUG_REPORT -> bugDescription + ReportType.SUGGESTION -> "[Suggestion] $bugDescription" + ReportType.SPACE_BETA_FEEDBACK -> "[spaces-feedback] $bugDescription" + ReportType.THREADS_BETA_FEEDBACK -> "[threads-feedback] $bugDescription" + ReportType.AUTO_UISI_SENDER, + ReportType.AUTO_UISI -> bugDescription + } - // build the multi part request - val builder = BugReporterMultipartBody.Builder() - .addFormDataPart("text", text) - .addFormDataPart("app", rageShakeAppNameForReport(reportType)) - // .addFormDataPart("user_agent", matrix.getUserAgent()) - .addFormDataPart("user_id", userId) - .addFormDataPart("can_contact", canContact.toString()) - .addFormDataPart("device_id", deviceId) - // .addFormDataPart("version", versionProvider.getVersion(longFormat = true)) - // .addFormDataPart("branch_name", buildMeta.gitBranchName) - // .addFormDataPart("matrix_sdk_version", Matrix.getSdkVersion()) - .addFormDataPart("olm_version", olmVersion) - .addFormDataPart("device", Build.MODEL.trim()) - // .addFormDataPart("verbose_log", vectorPreferences.labAllowedExtendedLogging().toOnOff()) - .addFormDataPart("multi_window", inMultiWindowMode.toOnOff()) - // .addFormDataPart( - // "os", Build.VERSION.RELEASE + " (API " + sdkIntProvider.get() + ") " + - // Build.VERSION.INCREMENTAL + "-" + Build.VERSION.CODENAME - // ) - .addFormDataPart("locale", Locale.getDefault().toString()) - // .addFormDataPart("app_language", vectorLocale.applicationLocale.toString()) - // .addFormDataPart("default_app_language", systemLocaleProvider.getSystemLocale().toString()) - // .addFormDataPart("theme", ThemeUtils.getApplicationTheme(context)) - .addFormDataPart("server_version", serverVersion) - .apply { - customFields?.forEach { (name, value) -> - addFormDataPart(name, value) - } + // build the multi part request + val builder = BugReporterMultipartBody.Builder() + .addFormDataPart("text", text) + .addFormDataPart("app", rageShakeAppNameForReport(reportType)) + // .addFormDataPart("user_agent", matrix.getUserAgent()) + .addFormDataPart("user_id", userId) + .addFormDataPart("can_contact", canContact.toString()) + .addFormDataPart("device_id", deviceId) + // .addFormDataPart("version", versionProvider.getVersion(longFormat = true)) + // .addFormDataPart("branch_name", buildMeta.gitBranchName) + // .addFormDataPart("matrix_sdk_version", Matrix.getSdkVersion()) + .addFormDataPart("olm_version", olmVersion) + .addFormDataPart("device", Build.MODEL.trim()) + // .addFormDataPart("verbose_log", vectorPreferences.labAllowedExtendedLogging().toOnOff()) + .addFormDataPart("multi_window", inMultiWindowMode.toOnOff()) + // .addFormDataPart( + // "os", Build.VERSION.RELEASE + " (API " + sdkIntProvider.get() + ") " + + // Build.VERSION.INCREMENTAL + "-" + Build.VERSION.CODENAME + // ) + .addFormDataPart("locale", Locale.getDefault().toString()) + // .addFormDataPart("app_language", vectorLocale.applicationLocale.toString()) + // .addFormDataPart("default_app_language", systemLocaleProvider.getSystemLocale().toString()) + // .addFormDataPart("theme", ThemeUtils.getApplicationTheme(context)) + .addFormDataPart("server_version", serverVersion) + .apply { + customFields?.forEach { (name, value) -> + addFormDataPart(name, value) } - - // add the gzipped files - for (file in gzippedFiles) { - builder.addFormDataPart("compressed-log", file.name, file.asRequestBody(MimeTypes.OctetStream.toMediaTypeOrNull())) } - mBugReportFiles.addAll(gzippedFiles) + // add the gzipped files + for (file in gzippedFiles) { + builder.addFormDataPart("compressed-log", file.name, file.asRequestBody(MimeTypes.OctetStream.toMediaTypeOrNull())) + } + + mBugReportFiles.addAll(gzippedFiles) - if (withScreenshot) { - screenshotHolder.getFileUri() - ?.toUri() - ?.toFile() - ?.let { screenshotFile -> - try { - builder.addFormDataPart( - "file", - screenshotFile.name, screenshotFile.asRequestBody(MimeTypes.OctetStream.toMediaTypeOrNull()) - ) - } catch (e: Exception) { - Timber.e(e, "## sendBugReport() : fail to write screenshot") - } + if (withScreenshot) { + screenshotHolder.getFileUri() + ?.toUri() + ?.toFile() + ?.let { screenshotFile -> + try { + builder.addFormDataPart( + "file", + screenshotFile.name, screenshotFile.asRequestBody(MimeTypes.OctetStream.toMediaTypeOrNull()) + ) + } catch (e: Exception) { + Timber.e(e, "## sendBugReport() : fail to write screenshot") } - } + } + } - // add some github labels - // builder.addFormDataPart("label", buildMeta.versionName) - // builder.addFormDataPart("label", buildMeta.flavorDescription) - // builder.addFormDataPart("label", buildMeta.gitBranchName) + // add some github labels + // builder.addFormDataPart("label", buildMeta.versionName) + // builder.addFormDataPart("label", buildMeta.flavorDescription) + // builder.addFormDataPart("label", buildMeta.gitBranchName) - // Possible values for BuildConfig.BUILD_TYPE: "debug", "nightly", "release". - // builder.addFormDataPart("label", BuildConfig.BUILD_TYPE) + // Possible values for BuildConfig.BUILD_TYPE: "debug", "nightly", "release". + // builder.addFormDataPart("label", BuildConfig.BUILD_TYPE) - when (reportType) { - ReportType.BUG_REPORT -> { - /* nop */ - } - ReportType.SUGGESTION -> builder.addFormDataPart("label", "[Suggestion]") - ReportType.SPACE_BETA_FEEDBACK -> builder.addFormDataPart("label", "spaces-feedback") - ReportType.THREADS_BETA_FEEDBACK -> builder.addFormDataPart("label", "threads-feedback") - ReportType.AUTO_UISI -> { - builder.addFormDataPart("label", "Z-UISI") - builder.addFormDataPart("label", "android") - builder.addFormDataPart("label", "uisi-recipient") - } - ReportType.AUTO_UISI_SENDER -> { - builder.addFormDataPart("label", "Z-UISI") - builder.addFormDataPart("label", "android") - builder.addFormDataPart("label", "uisi-sender") - } + when (reportType) { + ReportType.BUG_REPORT -> { + /* nop */ } - - if (crashCallStack.isNotEmpty() && withCrashLogs) { - builder.addFormDataPart("label", "crash") + ReportType.SUGGESTION -> builder.addFormDataPart("label", "[Suggestion]") + ReportType.SPACE_BETA_FEEDBACK -> builder.addFormDataPart("label", "spaces-feedback") + ReportType.THREADS_BETA_FEEDBACK -> builder.addFormDataPart("label", "threads-feedback") + ReportType.AUTO_UISI -> { + builder.addFormDataPart("label", "Z-UISI") + builder.addFormDataPart("label", "android") + builder.addFormDataPart("label", "uisi-recipient") } + ReportType.AUTO_UISI_SENDER -> { + builder.addFormDataPart("label", "Z-UISI") + builder.addFormDataPart("label", "android") + builder.addFormDataPart("label", "uisi-sender") + } + } - val requestBody = builder.build() - - // add a progress listener - requestBody.setWriteListener { totalWritten, contentLength -> - val percentage = if (-1L != contentLength) { - if (totalWritten > contentLength) { - 100 - } else { - (totalWritten * 100 / contentLength).toInt() - } - } else { - 0 - } + if (crashCallStack.isNotEmpty() && withCrashLogs) { + builder.addFormDataPart("label", "crash") + } - if (mIsCancelled && null != mBugReportCall) { - mBugReportCall!!.cancel() - } + val requestBody = builder.build() - Timber.v("## onWrite() : $percentage%") - try { - listener?.onProgress(percentage) - } catch (e: Exception) { - Timber.e(e, "## onProgress() : failed") + // add a progress listener + requestBody.setWriteListener { totalWritten, contentLength -> + val percentage = if (-1L != contentLength) { + if (totalWritten > contentLength) { + 100 + } else { + (totalWritten * 100 / contentLength).toInt() } + } else { + 0 } - // build the request - val request = Request.Builder() - .url(context.getString(R.string.bug_report_url)) - .post(requestBody) - .build() - - var responseCode = HttpURLConnection.HTTP_INTERNAL_ERROR - var response: Response? = null - var errorMessage: String? = null + if (mIsCancelled && null != mBugReportCall) { + mBugReportCall!!.cancel() + } - // trigger the request + Timber.v("## onWrite() : $percentage%") try { - mBugReportCall = okHttpClient.newCall(request) - response = mBugReportCall!!.execute() - responseCode = response.code + listener?.onProgress(percentage) } catch (e: Exception) { - Timber.e(e, "response") - errorMessage = e.localizedMessage + Timber.e(e, "## onProgress() : failed") } + } - // if the upload failed, try to retrieve the reason - if (responseCode != HttpURLConnection.HTTP_OK) { - if (null != errorMessage) { - serverError = "Failed with error $errorMessage" - } else if (response?.body == null) { - serverError = "Failed with error $responseCode" - } else { - try { - val inputStream = response.body!!.byteStream() - - serverError = inputStream.use { - buildString { - var ch = it.read() - while (ch != -1) { - append(ch.toChar()) - ch = it.read() - } - } - } + // build the request + val request = Request.Builder() + .url(context.getString(R.string.bug_report_url)) + .post(requestBody) + .build() + + var responseCode = HttpURLConnection.HTTP_INTERNAL_ERROR + var response: Response? = null + var errorMessage: String? = null + + // trigger the request + try { + mBugReportCall = okHttpClient.newCall(request) + response = mBugReportCall!!.execute() + responseCode = response.code + } catch (e: Exception) { + Timber.e(e, "response") + errorMessage = e.localizedMessage + } - // check if the error message - serverError?.let { - try { - val responseJSON = JSONObject(it) - serverError = responseJSON.getString("error") - } catch (e: JSONException) { - Timber.e(e, "doInBackground ; Json conversion failed") + // if the upload failed, try to retrieve the reason + if (responseCode != HttpURLConnection.HTTP_OK) { + if (null != errorMessage) { + serverError = "Failed with error $errorMessage" + } else if (response?.body == null) { + serverError = "Failed with error $responseCode" + } else { + try { + val inputStream = response.body!!.byteStream() + + serverError = inputStream.use { + buildString { + var ch = it.read() + while (ch != -1) { + append(ch.toChar()) + ch = it.read() } } + } - // should never happen - if (null == serverError) { - serverError = "Failed with error $responseCode" + // check if the error message + serverError?.let { + try { + val responseJSON = JSONObject(it) + serverError = responseJSON.getString("error") + } catch (e: JSONException) { + Timber.e(e, "doInBackground ; Json conversion failed") } - } catch (e: Exception) { - Timber.e(e, "## sendBugReport() : failed to parse error") } + + // should never happen + if (null == serverError) { + serverError = "Failed with error $responseCode" + } + } catch (e: Exception) { + Timber.e(e, "## sendBugReport() : failed to parse error") } - } else { - /* - reportURL = response?.body?.string()?.let { stringBody -> - adapter.fromJson(stringBody)?.get("report_url")?.toString() - } - */ } + } else { + /* + reportURL = response?.body?.string()?.let { stringBody -> + adapter.fromJson(stringBody)?.get("report_url")?.toString() + } + */ } } + } - withContext(coroutineDispatchers.main) { - mBugReportCall = null + withContext(coroutineDispatchers.main) { + mBugReportCall = null - // delete when the bug report has been successfully sent - for (file in mBugReportFiles) { - file.safeDelete() - } + // delete when the bug report has been successfully sent + for (file in mBugReportFiles) { + file.safeDelete() + } - if (null != listener) { - try { - if (mIsCancelled) { - listener.onUploadCancelled() - } else if (null == serverError) { - listener.onUploadSucceed(reportURL) - } else { - listener.onUploadFailed(serverError) - } - } catch (e: Exception) { - Timber.e(e, "## onPostExecute() : failed") + if (null != listener) { + try { + if (mIsCancelled) { + listener.onUploadCancelled() + } else if (null == serverError) { + listener.onUploadSucceed(reportURL) + } else { + listener.onUploadFailed(serverError) } + } catch (e: Exception) { + Timber.e(e, "## onPostExecute() : failed") } } } diff --git a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt index dd5d3e76c7..ac8940a1ac 100644 --- a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt +++ b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt @@ -20,13 +20,10 @@ import io.element.android.features.rageshake.api.reporter.BugReporter import io.element.android.features.rageshake.api.reporter.BugReporterListener import io.element.android.features.rageshake.api.reporter.ReportType import io.element.android.libraries.matrix.test.A_FAILURE_REASON -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay -import kotlinx.coroutines.launch class FakeBugReporter(val mode: FakeBugReporterMode = FakeBugReporterMode.Success) : BugReporter { - override fun sendBugReport( - coroutineScope: CoroutineScope, + override suspend fun sendBugReport( reportType: ReportType, withDevicesLogs: Boolean, withCrashLogs: Boolean, @@ -38,27 +35,25 @@ class FakeBugReporter(val mode: FakeBugReporterMode = FakeBugReporterMode.Succes customFields: Map?, listener: BugReporterListener?, ) { - coroutineScope.launch { - delay(100) - listener?.onProgress(0) - delay(100) - listener?.onProgress(50) - delay(100) - when (mode) { - FakeBugReporterMode.Success -> Unit - FakeBugReporterMode.Failure -> { - listener?.onUploadFailed(A_FAILURE_REASON) - return@launch - } - FakeBugReporterMode.Cancel -> { - listener?.onUploadCancelled() - return@launch - } + delay(100) + listener?.onProgress(0) + delay(100) + listener?.onProgress(50) + delay(100) + when (mode) { + FakeBugReporterMode.Success -> Unit + FakeBugReporterMode.Failure -> { + listener?.onUploadFailed(A_FAILURE_REASON) + return + } + FakeBugReporterMode.Cancel -> { + listener?.onUploadCancelled() + return } - listener?.onProgress(100) - delay(100) - listener?.onUploadSucceed(null) } + listener?.onProgress(100) + delay(100) + listener?.onUploadSucceed(null) } }