Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ class ZendeskEnvironmentDataSource @Inject constructor() {
"${selectedSite.hostURL} (${selectedSite.stateLogInformation})"
} ?: unknownHostValue

suspend fun getDeviceLogs() = WooLog.getCurrentLogEntries().joinToString("\n").takeLast(maxLogfileLength)
suspend fun getFullDeviceLogs() = WooLog.getCurrentLogEntries().joinToString("\n")

fun trimDeviceLogs(logs: String) = logs.takeLast(maxLogfileLength)

private val SiteModel.hostURL: String
get() = UrlUtils.removeScheme(url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.os.Parcelable
import com.woocommerce.android.applicationpasswords.IsAppPasswordsSupportedForJetpackSite
import com.woocommerce.android.extensions.formatResult
import com.woocommerce.android.support.help.HelpOrigin
import com.woocommerce.android.support.zendesk.RequestConstants.APPLICATION_LOG_FILENAME
import com.woocommerce.android.support.zendesk.RequestConstants.DIAGNOSTIC_LOG_FILENAME
import com.woocommerce.android.support.zendesk.RequestConstants.requestCreationIdentityNotSetErrorMessage
import com.woocommerce.android.support.zendesk.RequestConstants.requestCreationTimeoutErrorMessage
Expand All @@ -25,6 +26,7 @@ import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.launch
import kotlinx.coroutines.suspendCancellableCoroutine
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import kotlinx.parcelize.Parcelize
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.SiteStore
Expand Down Expand Up @@ -70,11 +72,12 @@ class ZendeskTicketRepository @Inject constructor(

val ssr: String? = selectedSite?.let { fetchSSR(it) }

val attachmentTokens = if (diagnosticLog != null) {
listOfNotNull(uploadDiagnosticLog(diagnosticLog))
} else {
emptyList()
}
val deviceLogs = envDataSource.getFullDeviceLogs()

val attachmentTokens = listOfNotNull(
diagnosticLog?.let { uploadTextAttachment(DIAGNOSTIC_LOG_FILENAME, it) },
uploadTextAttachment(APPLICATION_LOG_FILENAME, deviceLogs)
)
Comment thread
AdamGrzybkowski marked this conversation as resolved.

val requestCallback = object : ZendeskCallback<Request>() {
override fun onSuccess(result: Request?) {
Expand Down Expand Up @@ -103,7 +106,8 @@ class ZendeskTicketRepository @Inject constructor(
allSites = siteStore.sites,
selectedSite = selectedSite,
ssr = ssr,
siteAddress = siteAddress
siteAddress = siteAddress,
deviceLogs = deviceLogs
)

this.customFields = buildZendeskCustomFields(zendeskCustomFieldsParams)
Expand All @@ -130,37 +134,42 @@ class ZendeskTicketRepository @Inject constructor(
return result.model?.formatResult()
}

private suspend fun uploadDiagnosticLog(
diagnosticLog: String
private suspend fun uploadTextAttachment(
fileName: String,
content: String
): String? {
val tempFile = withContext(dispatchers.io) {
File.createTempFile(DIAGNOSTIC_LOG_FILENAME, null).also {
it.writeText(diagnosticLog)
File.createTempFile(fileName, null).also {
it.writeText(content)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we also make the local file preparation best-effort? File.createTempFile() and writeText() run before the guarded upload, so an I/O failure—for example, low disk space—would exit the flow before the Zendesk ticket is created. Since the application-log upload now runs for every request, an optional attachment could block contacting support entirely.
Wdyt if we catch file-preparation failures, log them, and continue without the attachment token?

}
}

return try {
suspendCancellableCoroutine { continuation ->
zendeskSettings.uploadProvider?.uploadAttachment(
DIAGNOSTIC_LOG_FILENAME,
tempFile,
"text/plain",
object : ZendeskCallback<UploadResponse>() {
override fun onSuccess(result: UploadResponse?) {
if (continuation.isActive) {
continuation.resume(result?.token)
// Guard against the Zendesk SDK never invoking either callback, which would otherwise
// block ticket creation indefinitely. On timeout we treat it as a failed upload (null).
withTimeoutOrNull(RequestConstants.attachmentUploadTimeout) {
suspendCancellableCoroutine { continuation ->
zendeskSettings.uploadProvider?.uploadAttachment(
fileName,
tempFile,
"text/plain",
object : ZendeskCallback<UploadResponse>() {
override fun onSuccess(result: UploadResponse?) {
if (continuation.isActive) {
continuation.resume(result?.token)
}
}
}

override fun onError(error: ErrorResponse?) {
wooLog.e(WooLog.T.SUPPORT, "Failed to upload diagnostic log")
if (continuation.isActive) {
continuation.resume(null)
override fun onError(error: ErrorResponse?) {
wooLog.e(WooLog.T.SUPPORT, "Failed to upload attachment: $fileName")
if (continuation.isActive) {
continuation.resume(null)
}
}
}
) ?: run {
continuation.resume(null)
}
) ?: run {
continuation.resume(null)
}
}
} finally {
Expand All @@ -177,7 +186,7 @@ class ZendeskTicketRepository @Inject constructor(
CustomField(TicketCustomField.appVersion, envDataSource.generateVersionName(params.context)),
CustomField(TicketCustomField.deviceFreeSpace, envDataSource.totalAvailableMemorySize),
CustomField(TicketCustomField.networkInformation, envDataSource.generateNetworkInformation(params.context)),
CustomField(TicketCustomField.logs, envDataSource.getDeviceLogs()),
CustomField(TicketCustomField.logs, envDataSource.trimDeviceLogs(params.deviceLogs)),
CustomField(TicketCustomField.ssr, params.ssr),
CustomField(TicketCustomField.currentSite, envDataSource.generateHostData(params.selectedSite)),
CustomField(TicketCustomField.sourcePlatform, ZendeskEnvironmentDataSource.sourcePlatform),
Expand Down Expand Up @@ -359,9 +368,11 @@ sealed class ZendeskException(message: String) : Exception(message) {

private object RequestConstants {
const val requestCreationTimeout = 10000L
const val attachmentUploadTimeout = 30000L
const val requestCreationTimeoutErrorMessage = "Request creation timed out"
const val requestCreationIdentityNotSetErrorMessage = "Request creation failed: identity not set"
const val DIAGNOSTIC_LOG_FILENAME = "connectivitytest_log.txt"
const val APPLICATION_LOG_FILENAME = "application_log.txt"
}

private data class ZendeskCustomFieldsParams(
Expand All @@ -370,5 +381,6 @@ private data class ZendeskCustomFieldsParams(
val allSites: List<SiteModel>?,
val selectedSite: SiteModel?,
val ssr: String?,
val siteAddress: String
val siteAddress: String,
val deviceLogs: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.junit.Test
import org.mockito.kotlin.any
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.eq
import org.mockito.kotlin.given
import org.mockito.kotlin.mock
import org.mockito.kotlin.stub
Expand All @@ -40,6 +41,8 @@ import org.wordpress.android.fluxc.store.SiteStore
import zendesk.support.CreateRequest
import zendesk.support.Request
import zendesk.support.RequestProvider
import zendesk.support.UploadProvider
import zendesk.support.UploadResponse

@OptIn(ExperimentalCoroutinesApi::class)
internal class ZendeskTicketRepositoryTest : BaseUnitTest() {
Expand Down Expand Up @@ -728,6 +731,158 @@ internal class ZendeskTicketRepositoryTest : BaseUnitTest() {
assertThat(tags).contains(ZendeskTags.jetpackSiteUsingAppPasswords)
}

@Test
fun `when createRequest is called, then the full device logs are uploaded as an attachment and its token is set`() =
testBlocking {
// given
val expectedToken = "attachment-token"
Comment thread
AdamGrzybkowski marked this conversation as resolved.
val uploadResponse = mock<UploadResponse> { on { token } doReturn expectedToken }
val uploadProvider = mock<UploadProvider>()
given(zendeskSettings.uploadProvider).willReturn(uploadProvider)
val uploadCaptor = argumentCaptor<ZendeskCallback<UploadResponse>>()
val requestCaptor = argumentCaptor<CreateRequest>()

// when
val job = launch {
sut.createRequest(
context = mock(),
origin = HelpOrigin.LOGIN_HELP_NOTIFICATION,
ticketType = TicketType.MobileApp,
selectedSite = null,
subject = "subject",
description = "description",
extraTags = emptyList(),
siteAddress = "siteAddress"
).first()
}

// then
verify(uploadProvider).uploadAttachment(
eq("application_log.txt"),
any(),
eq("text/plain"),
uploadCaptor.capture()
)
uploadCaptor.firstValue.onSuccess(uploadResponse)
advanceUntilIdle()
verify(requestProvider).createRequest(requestCaptor.capture(), any())
job.cancel()

assertThat(requestCaptor.firstValue.attachments).containsExactly(expectedToken)
}

@Test
fun `given the log upload fails, when createRequest is called, then the request is still created without attachments`() =
testBlocking {
// given
val uploadProvider = mock<UploadProvider>()
given(zendeskSettings.uploadProvider).willReturn(uploadProvider)
val uploadCaptor = argumentCaptor<ZendeskCallback<UploadResponse>>()
val requestCaptor = argumentCaptor<CreateRequest>()

// when
val job = launch {
sut.createRequest(
context = mock(),
origin = HelpOrigin.LOGIN_HELP_NOTIFICATION,
ticketType = TicketType.MobileApp,
selectedSite = null,
subject = "subject",
description = "description",
extraTags = emptyList(),
siteAddress = "siteAddress"
).first()
}

// then
verify(uploadProvider).uploadAttachment(any(), any(), any(), uploadCaptor.capture())
uploadCaptor.firstValue.onError(mock())
advanceUntilIdle()
verify(requestProvider).createRequest(requestCaptor.capture(), any())
job.cancel()

assertThat(requestCaptor.firstValue.attachments).isNullOrEmpty()
}

@Test
fun `given the log upload never returns, when createRequest is called, then it times out and creates the request`() =
testBlocking {
// given the upload provider never invokes either callback
val uploadProvider = mock<UploadProvider>()
given(zendeskSettings.uploadProvider).willReturn(uploadProvider)
val requestCaptor = argumentCaptor<CreateRequest>()

// when
val job = launch {
sut.createRequest(
context = mock(),
origin = HelpOrigin.LOGIN_HELP_NOTIFICATION,
ticketType = TicketType.MobileApp,
selectedSite = null,
subject = "subject",
description = "description",
extraTags = emptyList(),
siteAddress = "siteAddress"
).first()
}

// then the upload times out, is treated as no attachment, and the ticket is still created
verify(uploadProvider).uploadAttachment(eq("application_log.txt"), any(), any(), any())
advanceUntilIdle()
verify(requestProvider).createRequest(requestCaptor.capture(), any())
job.cancel()

assertThat(requestCaptor.firstValue.attachments).isNullOrEmpty()
}

@Test
fun `given a diagnostic log, when createRequest is called, then both the diagnostic and full logs are attached`() =
testBlocking {
// given
val diagnosticResponse = mock<UploadResponse> { on { token } doReturn "diagnostic-token" }
val appLogResponse = mock<UploadResponse> { on { token } doReturn "app-log-token" }
val uploadProvider = mock<UploadProvider>()
given(zendeskSettings.uploadProvider).willReturn(uploadProvider)
val uploadCaptor = argumentCaptor<ZendeskCallback<UploadResponse>>()
val requestCaptor = argumentCaptor<CreateRequest>()

// when
val job = launch {
sut.createRequest(
context = mock(),
origin = HelpOrigin.LOGIN_HELP_NOTIFICATION,
ticketType = TicketType.MobileApp,
selectedSite = null,
subject = "subject",
description = "description",
extraTags = emptyList(),
siteAddress = "siteAddress",
diagnosticLog = "diagnostic logs"
).first()
}

// then
verify(uploadProvider).uploadAttachment(
eq("connectivitytest_log.txt"),
any(),
any(),
uploadCaptor.capture()
)
uploadCaptor.firstValue.onSuccess(diagnosticResponse)
verify(uploadProvider).uploadAttachment(
eq("application_log.txt"),
any(),
any(),
uploadCaptor.capture()
)
uploadCaptor.secondValue.onSuccess(appLogResponse)
advanceUntilIdle()
verify(requestProvider).createRequest(requestCaptor.capture(), any())
job.cancel()

assertThat(requestCaptor.firstValue.attachments).containsExactly("diagnostic-token", "app-log-token")
}

private fun createSUT() {
sut = ZendeskTicketRepository(
zendeskSettings = zendeskSettings,
Expand All @@ -743,7 +898,8 @@ internal class ZendeskTicketRepositoryTest : BaseUnitTest() {
private fun mockEnvDataSource() = mock<ZendeskEnvironmentDataSource> {
on { totalAvailableMemorySize } doReturn "100"
on { deviceLanguage } doReturn "testLanguage"
on { getDeviceLogs() } doReturn "logs"
on { getFullDeviceLogs() } doReturn "full logs"
on { trimDeviceLogs(any()) } doReturn "logs"
on { generateVersionName(any()) } doReturn "version"
on { generateNetworkInformation(any()) } doReturn "networkInfo"
on { generateCombinedLogInformationOfSites(any()) } doReturn "sitesInfo"
Expand Down
Loading