Skip to content

Commit 9b340a7

Browse files
FranAguileraFranAguilera
authored
[Android] Hardening Issue Reporting and log internal errors (#911)
1 parent 09e59bd commit 9b340a7

File tree

10 files changed

+235
-225
lines changed

10 files changed

+235
-225
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
**Added**
2323

24-
- Nothing yet!
24+
- Log internal sdk error if we fail to process and persist other issue reports (e.g. Native Crash, ANRs, etc)
2525

2626
**Changed**
2727

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import io.bitdrift.capture.providers.Field
1414
import io.bitdrift.capture.providers.session.SessionStrategyConfiguration
1515
import io.bitdrift.capture.reports.processor.IStreamingReportProcessor
1616
import io.bitdrift.capture.reports.processor.ReportProcessingSession
17+
import okio.IOException
1718
import java.io.InputStream
1819

1920
// We use our own type here instead of a builtin function to allow us to avoid proguard-rewriting this class.
@@ -370,6 +371,7 @@ internal object CaptureJniLibrary : IBridge, IStreamingReportProcessor {
370371
* @param timestampMillis The time at which the event took place
371372
* @param destinationPath Target file path to write the report
372373
*/
374+
@Throws(IOException::class, IllegalArgumentException::class)
373375
external override fun processAndPersistANR(
374376
stream: InputStream,
375377
timestampMillis: Long,

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IInternalLogger.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ internal interface IInternalLogger : ILogger {
3434
message: () -> String,
3535
)
3636

37+
fun logInternalError(
38+
throwable: Throwable? = null,
39+
blocking: Boolean = false,
40+
message: () -> String,
41+
)
42+
3743
fun handleInternalError(
3844
detail: String,
3945
throwable: Throwable? = null,

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,22 @@ internal class LoggerImpl(
470470
}
471471
}
472472

473+
override fun logInternalError(
474+
throwable: Throwable?,
475+
blocking: Boolean,
476+
message: () -> String,
477+
) {
478+
logInternal(
479+
type = LogType.INTERNALSDK,
480+
level = LogLevel.ERROR,
481+
arrayFields = ArrayFields.EMPTY,
482+
throwable = throwable,
483+
blocking = blocking,
484+
) {
485+
message()
486+
}
487+
}
488+
473489
override fun logInternal(
474490
type: LogType,
475491
level: LogLevel,

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/reports/IssueReporter.kt‎

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@ import androidx.annotation.VisibleForTesting
1414
import io.bitdrift.capture.Capture.LOG_TAG
1515
import io.bitdrift.capture.CaptureJniLibrary
1616
import io.bitdrift.capture.IInternalLogger
17-
import io.bitdrift.capture.LogLevel
18-
import io.bitdrift.capture.LogType
1917
import io.bitdrift.capture.attributes.IClientAttributes
2018
import io.bitdrift.capture.common.IBackgroundThreadHandler
21-
import io.bitdrift.capture.providers.ArrayFields
2219
import io.bitdrift.capture.providers.DateProvider
2320
import io.bitdrift.capture.reports.IssueReporterState.NotInitialized
2421
import io.bitdrift.capture.reports.IssueReporterState.RuntimeState
@@ -55,8 +52,6 @@ internal class IssueReporter(
5552
private val latestAppExitInfoProvider: ILatestAppExitInfoProvider = LatestAppExitInfoProvider,
5653
private val captureUncaughtExceptionHandler: ICaptureUncaughtExceptionHandler = CaptureUncaughtExceptionHandler,
5754
private val dateProvider: DateProvider,
58-
private val issueReporterProcessorFactory: (String, IClientAttributes, DateProvider) -> IIssueReporterProcessor =
59-
::buildDefaultIssueReporterProcessor,
6055
) : IIssueReporter,
6156
IJvmCrashListener {
6257
@VisibleForTesting
@@ -92,7 +87,7 @@ internal class IssueReporter(
9287
}
9388

9489
runCatching {
95-
issueReporterProcessor = issueReporterProcessorFactory(sdkDirectory, clientAttributes, dateProvider)
90+
issueReporterProcessor = buildDefaultIssueReporterProcessor(sdkDirectory, clientAttributes, dateProvider, internalLogger)
9691
captureUncaughtExceptionHandler.install(this)
9792
processPriorReports(activityManager, completedReportsProcessor)
9893
}.getOrElse {
@@ -120,25 +115,11 @@ internal class IssueReporter(
120115
thread: Thread,
121116
throwable: Throwable,
122117
) {
123-
runCatching {
124-
issueReporterProcessor?.processJvmCrash(
125-
callerThread = thread,
126-
throwable = throwable,
127-
allThreads = Thread.getAllStackTraces(),
128-
)
129-
}.getOrElse {
130-
val errorMessage = "Error while processing JVM crash"
131-
internalLogger.logInternal(
132-
type = LogType.INTERNALSDK,
133-
level = LogLevel.ERROR,
134-
arrayFields = ArrayFields.EMPTY,
135-
throwable = it,
136-
blocking = true,
137-
) {
138-
errorMessage
139-
}
140-
Log.e(LOG_TAG, "$errorMessage. $it")
141-
}
118+
issueReporterProcessor?.processJvmCrash(
119+
callerThread = thread,
120+
throwable = throwable,
121+
allThreads = Thread.getAllStackTraces(),
122+
)
142123
}
143124

144125
override fun getLogStatusFieldsMap(): Map<String, String> =
@@ -231,12 +212,14 @@ internal class IssueReporter(
231212
sdkDirectory: String,
232213
clientAttributes: IClientAttributes,
233214
dateProvider: DateProvider,
215+
internalLogger: IInternalLogger,
234216
): IIssueReporterProcessor =
235217
IssueReporterProcessor(
236218
IssueReporterStore(sdkDirectory),
237219
clientAttributes,
238220
CaptureJniLibrary,
239221
dateProvider,
222+
internalLogger,
240223
)
241224
}
242225
}

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/reports/processor/IssueReporterProcessor.kt‎

Lines changed: 73 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@ package io.bitdrift.capture.reports.processor
99

1010
import android.os.Build
1111
import android.os.strictmode.Violation
12-
import android.util.Log
1312
import androidx.annotation.RequiresApi
1413
import com.google.flatbuffers.FlatBufferBuilder
1514
import io.bitdrift.capture.BuildConstants
16-
import io.bitdrift.capture.Capture.LOG_TAG
15+
import io.bitdrift.capture.IInternalLogger
1716
import io.bitdrift.capture.attributes.ClientAttributes
1817
import io.bitdrift.capture.attributes.IClientAttributes
1918
import io.bitdrift.capture.providers.DateProvider
@@ -39,6 +38,7 @@ internal class IssueReporterProcessor(
3938
private val clientAttributes: IClientAttributes,
4039
private val streamingReportsProcessor: IStreamingReportProcessor,
4140
private val dateProvider: DateProvider,
41+
private val internalLogger: IInternalLogger,
4242
) : IIssueReporterProcessor {
4343
companion object {
4444
// Initial size for file builder buffer
@@ -86,7 +86,11 @@ internal class IssueReporterProcessor(
8686
sdkVersion = sdkVersion,
8787
)
8888
}.onFailure {
89-
Log.e(LOG_TAG, "Error at persistJavaScriptReport: $it", it)
89+
internalLogger.logInternalError(
90+
throwable = it,
91+
) {
92+
"Error while persisting and processing a JavaScriptError"
93+
}
9094
}
9195
}
9296

@@ -104,34 +108,40 @@ internal class IssueReporterProcessor(
104108
description: String?,
105109
traceInputStream: InputStream,
106110
) {
107-
if (fatalIssueType == ReportType.AppNotResponding) {
108-
streamingReportsProcessor.processAndPersistANR(
109-
traceInputStream,
110-
timestamp,
111-
reporterIssueStore.generateFatalIssueFilePath(),
112-
clientAttributes,
113-
)
114-
} else if (fatalIssueType == ReportType.NativeCrash) {
115-
val builder = FlatBufferBuilder(FBS_BUILDER_DEFAULT_SIZE)
116-
val sdk = createSDKInfo(builder)
117-
val appMetrics = createAppMetrics(builder)
118-
val deviceMetrics = createDeviceMetrics(builder, timestamp)
119-
val report =
120-
NativeCrashProcessor.process(
121-
builder,
122-
sdk,
123-
appMetrics,
124-
deviceMetrics,
125-
description,
111+
runCatching {
112+
if (fatalIssueType == ReportType.AppNotResponding) {
113+
streamingReportsProcessor.processAndPersistANR(
126114
traceInputStream,
115+
timestamp,
116+
reporterIssueStore.generateFatalIssueFilePath(),
117+
clientAttributes,
127118
)
128-
builder.finish(report)
119+
} else if (fatalIssueType == ReportType.NativeCrash) {
120+
val builder = FlatBufferBuilder(FBS_BUILDER_DEFAULT_SIZE)
121+
val sdk = createSDKInfo(builder)
122+
val appMetrics = createAppMetrics(builder)
123+
val deviceMetrics = createDeviceMetrics(builder, timestamp)
124+
val report =
125+
NativeCrashProcessor.process(
126+
builder,
127+
sdk,
128+
appMetrics,
129+
deviceMetrics,
130+
description,
131+
traceInputStream,
132+
)
133+
builder.finish(report)
129134

130-
reporterIssueStore.persistFatalIssue(
131-
timestamp,
132-
builder.sizedByteArray(),
133-
ReportType.NativeCrash,
134-
)
135+
reporterIssueStore.persistFatalIssue(
136+
timestamp,
137+
builder.sizedByteArray(),
138+
ReportType.NativeCrash,
139+
)
140+
}
141+
}.onFailure {
142+
internalLogger.logInternalError(it) {
143+
"Error while processing and persisting an AppExit report"
144+
}
135145
}
136146
}
137147

@@ -177,36 +187,42 @@ internal class IssueReporterProcessor(
177187
reportType: Byte,
178188
isFatal: Boolean,
179189
) {
180-
val timestamp = dateProvider.invoke().time
181-
val builder = FlatBufferBuilder(FBS_BUILDER_DEFAULT_SIZE)
182-
val sdk = createSDKInfo(builder)
183-
val appMetrics = createAppMetrics(builder)
184-
val deviceMetrics = createDeviceMetrics(builder, timestamp)
185-
val report =
186-
JvmProcessor.getJvmReport(
187-
builder,
188-
sdk,
189-
appMetrics,
190-
deviceMetrics,
191-
throwable,
192-
callerThread,
193-
allThreads,
194-
reportType,
195-
)
196-
builder.finish(report)
190+
runCatching {
191+
val timestamp = dateProvider.invoke().time
192+
val builder = FlatBufferBuilder(FBS_BUILDER_DEFAULT_SIZE)
193+
val sdk = createSDKInfo(builder)
194+
val appMetrics = createAppMetrics(builder)
195+
val deviceMetrics = createDeviceMetrics(builder, timestamp)
196+
val report =
197+
JvmProcessor.getJvmReport(
198+
builder,
199+
sdk,
200+
appMetrics,
201+
deviceMetrics,
202+
throwable,
203+
callerThread,
204+
allThreads,
205+
reportType,
206+
)
207+
builder.finish(report)
197208

198-
if (isFatal) {
199-
reporterIssueStore.persistFatalIssue(
200-
timestamp,
201-
builder.sizedByteArray(),
202-
reportType,
203-
)
204-
} else {
205-
reporterIssueStore.persistNonFatalIssue(
206-
timestamp,
207-
builder.sizedByteArray(),
208-
reportType,
209-
)
209+
if (isFatal) {
210+
reporterIssueStore.persistFatalIssue(
211+
timestamp,
212+
builder.sizedByteArray(),
213+
reportType,
214+
)
215+
} else {
216+
reporterIssueStore.persistNonFatalIssue(
217+
timestamp,
218+
builder.sizedByteArray(),
219+
reportType,
220+
)
221+
}
222+
}.onFailure {
223+
internalLogger.logInternalError(it, blocking = isFatal) {
224+
"Error while processing and persisting a JVM report"
225+
}
210226
}
211227
}
212228

‎platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/fakes/FakeIssueReporterProcessor.kt‎

Lines changed: 0 additions & 63 deletions
This file was deleted.

0 commit comments

Comments
 (0)