Skip to content

Commit

Permalink
Merge pull request #1823 from embrace-io/testing-tweaks
Browse files Browse the repository at this point in the history
Tweak log attachment implementation after manual testing
  • Loading branch information
fractalwrench authored Jan 17, 2025
2 parents f996835 + eb9e17e commit 0cf563c
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ sealed class Attachment(val id: String) {
}

abstract val attributes: Map<EmbraceAttributeKey, String>
abstract val errorCode: AttachmentErrorCode?

protected fun constructAttributes(
id: String,
Expand All @@ -42,7 +43,7 @@ sealed class Attachment(val id: String) {

private val size: Long = bytes.size.toLong()

private val errorCode: AttachmentErrorCode? = when {
override val errorCode: AttachmentErrorCode? = when {
!counter() -> OVER_MAX_ATTACHMENTS
size > LIMIT_MB -> ATTACHMENT_TOO_LARGE
else -> null
Expand All @@ -64,7 +65,7 @@ sealed class Attachment(val id: String) {
counter: () -> Boolean,
) : Attachment(id) {

private val errorCode: AttachmentErrorCode? = when {
override val errorCode: AttachmentErrorCode? = when {
!counter() -> OVER_MAX_ATTACHMENTS
url.isEmpty() -> UNKNOWN
isNotUuid() -> UNKNOWN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ enum class SupportedEnvelopeType(

CRASH(Envelope.logEnvelopeType, "p1", Endpoint.LOGS),
SESSION(Envelope.sessionEnvelopeType, "p3", Endpoint.SESSIONS),
ATTACHMENT(null, "p4", Endpoint.ATTACHMENT),
ATTACHMENT(null, "p4", Endpoint.ATTACHMENTS),
LOG(Envelope.logEnvelopeType, "p5", Endpoint.LOGS),
BLOB(Envelope.logEnvelopeType, "p7", Endpoint.LOGS);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class OkHttpRequestExecutionService(
envelopeType: SupportedEnvelopeType,
payloadType: String,
): ExecutionResult {
val multipart = envelopeType.endpoint == Endpoint.ATTACHMENT
val multipart = envelopeType.endpoint == Endpoint.ATTACHMENTS
val apiRequest = envelopeType.endpoint.getApiRequestFromEndpoint(multipart)
val request = when {
multipart -> prepareMultipartRequest(payloadStream, apiRequest)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ enum class Endpoint(
LOGS("logs", "v2"),
SESSIONS("spans", "v2"),
CONFIG("config", "v2"),
ATTACHMENT("attachment", "v2"),
ATTACHMENTS("attachments", "v2"),
UNKNOWN("unknown", "v1"),
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal class FakeApiServer(
deliveryTracer.onServerReceivedRequest(endpoint.name)
return when (endpoint) {
Endpoint.LOGS, Endpoint.SESSIONS -> handleEnvelopeRequest(request, endpoint)
Endpoint.ATTACHMENT -> handleAttachmentRequest(request)
Endpoint.ATTACHMENTS -> handleAttachmentRequest(request)

// IMPORTANT NOTE: this response is not used until the SDK next starts!
Endpoint.CONFIG -> handleConfigRequest(request)
Expand Down Expand Up @@ -150,7 +150,7 @@ internal class FakeApiServer(
"logs" -> Endpoint.LOGS
"spans" -> Endpoint.SESSIONS
"config" -> Endpoint.CONFIG
"attachment" -> Endpoint.ATTACHMENT
"attachments" -> Endpoint.ATTACHMENTS
else -> error("Unsupported path $endpoint")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import io.embrace.android.embracesdk.internal.injection.ModuleInitBootstrapper
import io.embrace.android.embracesdk.internal.injection.embraceImplInject
import io.embrace.android.embracesdk.internal.logs.attachments.Attachment
import io.embrace.android.embracesdk.internal.logs.attachments.Attachment.EmbraceHosted
import io.embrace.android.embracesdk.internal.logs.attachments.AttachmentErrorCode.ATTACHMENT_TOO_LARGE
import io.embrace.android.embracesdk.internal.logs.attachments.AttachmentErrorCode.OVER_MAX_ATTACHMENTS
import io.embrace.android.embracesdk.internal.logs.attachments.AttachmentErrorCode.UNKNOWN
import io.embrace.android.embracesdk.internal.payload.PushNotificationBreadcrumb
import io.embrace.android.embracesdk.internal.serialization.truncatedStacktrace
import io.embrace.android.embracesdk.internal.utils.getSafeStackTrace
Expand All @@ -31,6 +34,9 @@ internal class LogsApiDelegate(
private val attachmentService by embraceImplInject(sdkCallChecker) {
bootstrapper.logModule.attachmentService
}
private val logger by embraceImplInject(sdkCallChecker) {
bootstrapper.initModule.logger
}

override fun logInfo(message: String) = logMessage(message, Severity.INFO)
override fun logWarning(message: String) = logMessage(message, Severity.WARNING)
Expand Down Expand Up @@ -91,6 +97,7 @@ internal class LogsApiDelegate(
attachment: ByteArray,
) {
val obj = attachmentService?.createAttachment(attachment) ?: return
logAttachmentErrorIfNeeded(obj)
logMessageImpl(
severity = severity,
message = message,
Expand All @@ -107,6 +114,7 @@ internal class LogsApiDelegate(
attachmentUrl: String,
) {
val obj = attachmentService?.createAttachment(attachmentId, attachmentUrl) ?: return
logAttachmentErrorIfNeeded(obj)
logMessageImpl(
severity = severity,
message = message,
Expand All @@ -115,6 +123,18 @@ internal class LogsApiDelegate(
)
}

private fun logAttachmentErrorIfNeeded(obj: Attachment) {
if (obj.errorCode != null) {
val msg = when (obj.errorCode) {
ATTACHMENT_TOO_LARGE -> "Supplied attachment exceeds 1Mb limit. This attachment will not be uploaded."
OVER_MAX_ATTACHMENTS -> "A maximum of 5 attachments are allowed per session. This attachment will not be uploaded."
UNKNOWN -> "An unknown error occurred while processing the attachment."
null -> null

Check warning on line 132 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/LogsApiDelegate.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/LogsApiDelegate.kt#L131-L132

Added lines #L131 - L132 were not covered by tests
} ?: return
logger?.logError(msg, RuntimeException(msg))
}
}

override fun logException(
throwable: Throwable,
severity: Severity,
Expand Down

0 comments on commit 0cf563c

Please sign in to comment.