Skip to content

Commit e324046

Browse files
mrobermrober
authored
Implement on-demand fatals internally (#3402)
* Prototype of on-demand fatals. * WIP - Add on demand count to internal keys * WIP - add rate limiting report queue before data transport. The blocking param is temp, I need to do some refactoring to make it never block for on-demand * WIP - Read queue params from settings. * WIP - Clean up Tasks a bit. Fixed tests. Added .m2 repo. * WIP - Sync with main, update pom files to work with firebase bom * WIP - Updated recorded and added dropped on-demand counts to own class * WIP - Plumbed the on-demand counter through properly, and added missing copyright text * Remove artifact files, make new internal api private, and pull in latest changes from main branch. This should be in a good state to merge after this. We need to followup with a change to create a FirebaseCrashlyticsInternal interface with the new internal api. * Resolve comments. Revert FirebaseCrashlytics api change, the plugin will call from core directly.
1 parent 2bf81a5 commit e324046

File tree

14 files changed

+444
-48
lines changed

14 files changed

+444
-48
lines changed

‎firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,13 @@ public void onReportSend_successfulReportsAreDeleted() {
438438
final Task<CrashlyticsReportWithSessionId> failedTask =
439439
Tasks.forException(new Exception("fail"));
440440

441-
when(reportSender.sendReport(mockReport1)).thenReturn(successfulTask);
442-
when(reportSender.sendReport(mockReport2)).thenReturn(failedTask);
441+
when(reportSender.enqueueReport(mockReport1, false)).thenReturn(successfulTask);
442+
when(reportSender.enqueueReport(mockReport2, false)).thenReturn(failedTask);
443443

444444
reportingCoordinator.sendReports(Runnable::run);
445445

446-
verify(reportSender).sendReport(mockReport1);
447-
verify(reportSender).sendReport(mockReport2);
446+
verify(reportSender).enqueueReport(mockReport1, false);
447+
verify(reportSender).enqueueReport(mockReport2, false);
448448
}
449449

450450
@Test

‎firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/send/DataTransportCrashlyticsReportSenderTest.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.android.gms.tasks.Task;
3030
import com.google.android.gms.tasks.Tasks;
3131
import com.google.firebase.crashlytics.internal.common.CrashlyticsReportWithSessionId;
32+
import com.google.firebase.crashlytics.internal.common.OnDemandCounter;
3233
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport;
3334
import java.io.File;
3435
import java.util.concurrent.ExecutionException;
@@ -51,7 +52,9 @@ public class DataTransportCrashlyticsReportSenderTest {
5152
public void setUp() throws Exception {
5253
MockitoAnnotations.initMocks(this);
5354
when(mockTransform.apply(any())).thenReturn(new byte[0]);
54-
reportSender = new DataTransportCrashlyticsReportSender(mockTransport, mockTransform);
55+
reportSender =
56+
new DataTransportCrashlyticsReportSender(
57+
new ReportQueue(60, 1.2, 3_000, mockTransport, new OnDemandCounter()), mockTransform);
5558
}
5659

5760
@Test
@@ -61,10 +64,13 @@ public void testSendReportsSuccessful() throws Exception {
6164
final CrashlyticsReportWithSessionId report1 = mockReportWithSessionId();
6265
final CrashlyticsReportWithSessionId report2 = mockReportWithSessionId();
6366

64-
final Task<CrashlyticsReportWithSessionId> send1 = reportSender.sendReport(report1);
65-
final Task<CrashlyticsReportWithSessionId> send2 = reportSender.sendReport(report2);
67+
final Task<CrashlyticsReportWithSessionId> send1 =
68+
reportSender.enqueueReport(report1, /*isOnDemand=*/ true);
69+
final Task<CrashlyticsReportWithSessionId> send2 =
70+
reportSender.enqueueReport(report2, /*isOnDemand=*/ true);
6671

6772
try {
73+
Thread.sleep(2_000); // give time to process queue
6874
Tasks.await(send1);
6975
Tasks.await(send2);
7076
} catch (ExecutionException e) {
@@ -85,10 +91,11 @@ public void testSendReportsFailure() throws Exception {
8591
final CrashlyticsReportWithSessionId report1 = mockReportWithSessionId();
8692
final CrashlyticsReportWithSessionId report2 = mockReportWithSessionId();
8793

88-
final Task<CrashlyticsReportWithSessionId> send1 = reportSender.sendReport(report1);
89-
final Task<CrashlyticsReportWithSessionId> send2 = reportSender.sendReport(report2);
94+
final Task<CrashlyticsReportWithSessionId> send1 = reportSender.enqueueReport(report1, false);
95+
final Task<CrashlyticsReportWithSessionId> send2 = reportSender.enqueueReport(report2, false);
9096

9197
try {
98+
Thread.sleep(2_000); // give time to process queue
9299
Tasks.await(send1);
93100
Tasks.await(send2);
94101
} catch (ExecutionException e) {
@@ -112,10 +119,11 @@ public void testSendReports_oneSuccessOneFail() throws Exception {
112119
final CrashlyticsReportWithSessionId report1 = mockReportWithSessionId();
113120
final CrashlyticsReportWithSessionId report2 = mockReportWithSessionId();
114121

115-
final Task<CrashlyticsReportWithSessionId> send1 = reportSender.sendReport(report1);
116-
final Task<CrashlyticsReportWithSessionId> send2 = reportSender.sendReport(report2);
122+
final Task<CrashlyticsReportWithSessionId> send1 = reportSender.enqueueReport(report1, false);
123+
final Task<CrashlyticsReportWithSessionId> send2 = reportSender.enqueueReport(report2, false);
117124

118125
try {
126+
Thread.sleep(2_000); // give time to process queue
119127
Tasks.await(send1);
120128
Tasks.await(send2);
121129
} catch (ExecutionException e) {

‎firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/TestSettingsData.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ public TestSettingsData(
3737
buildSettingsData(),
3838
buildFeaturesData(),
3939
settingsVersion,
40-
3600);
40+
3600,
41+
10,
42+
1.2,
43+
60);
4144
}
4245

4346
private static FeaturesSettingsData buildFeaturesData() {

‎firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ class CrashlyticsController {
8585
private final SessionReportingCoordinator reportingCoordinator;
8686

8787
private CrashlyticsUncaughtExceptionHandler crashHandler;
88+
private SettingsDataProvider settingsProvider = null;
8889

8990
// A promise that will be resolved when unsent reports are found on the device, and
9091
// send/deleteUnsentReports can be called to decide how to deal with them.
@@ -140,6 +141,7 @@ void enableExceptionHandling(
140141
String sessionIdentifier,
141142
Thread.UncaughtExceptionHandler defaultHandler,
142143
SettingsDataProvider settingsProvider) {
144+
this.settingsProvider = settingsProvider;
143145
// This must be called before installing the controller with
144146
// Thread.setDefaultUncaughtExceptionHandler to ensure that we are ready to handle
145147
// any crashes we catch.
@@ -160,10 +162,18 @@ public void onUncaughtException(
160162
Thread.setDefaultUncaughtExceptionHandler(crashHandler);
161163
}
162164

163-
synchronized void handleUncaughtException(
165+
void handleUncaughtException(
164166
@NonNull SettingsDataProvider settingsDataProvider,
165167
@NonNull final Thread thread,
166168
@NonNull final Throwable ex) {
169+
handleUncaughtException(settingsDataProvider, thread, ex, /*isOnDemand=*/ false);
170+
}
171+
172+
synchronized void handleUncaughtException(
173+
@NonNull SettingsDataProvider settingsDataProvider,
174+
@NonNull final Thread thread,
175+
@NonNull final Throwable ex,
176+
boolean isOnDemand) {
167177

168178
Logger.getLogger()
169179
.d("Handling uncaught " + "exception \"" + ex + "\" from thread " + thread.getName());
@@ -222,13 +232,15 @@ public Task<Void> then(@Nullable AppSettingsData appSettingsData)
222232
// Data collection is enabled, so it's safe to send the report.
223233
return Tasks.whenAll(
224234
logAnalyticsAppExceptionEvents(),
225-
reportingCoordinator.sendReports(executor));
235+
reportingCoordinator.sendReports(
236+
executor, isOnDemand ? currentSessionId : null));
226237
}
227238
});
228239
}
229240
});
230241

231242
try {
243+
// TODO(mrober): Don't block the main thread ever for on-demand fatals.
232244
Utils.awaitEvenIfOnMainThread(handleUncaughtExceptionTask);
233245
} catch (Exception e) {
234246
Logger.getLogger().e("Error handling uncaught exception", e);
@@ -419,6 +431,14 @@ public void run() {
419431
});
420432
}
421433

434+
void logFatalException(Thread thread, Throwable ex) {
435+
if (settingsProvider == null) {
436+
Logger.getLogger().w("settingsProvider not set");
437+
return;
438+
}
439+
handleUncaughtException(settingsProvider, thread, ex, /*isOnDemand=*/ true);
440+
}
441+
422442
void setUserId(String identifier) {
423443
userMetadata.setUserId(identifier);
424444
}

‎firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,19 @@ public class CrashlyticsCore {
6161

6262
static final int DEFAULT_MAIN_HANDLER_TIMEOUT_SEC = 4;
6363

64+
private static final String ON_DEMAND_RECORDED_KEY =
65+
"com.crashlytics.on-demand.recorded-exceptions";
66+
private static final String ON_DEMAND_DROPPED_KEY =
67+
"com.crashlytics.on-demand.dropped-exceptions";
68+
6469
// If this marker sticks around, the app is crashing before we finished initializing
6570
private static final String INITIALIZATION_MARKER_FILE_NAME = "initialization_marker";
6671
static final String CRASH_MARKER_FILE_NAME = "crash_marker";
6772

6873
private final Context context;
6974
private final FirebaseApp app;
7075
private final DataCollectionArbiter dataCollectionArbiter;
76+
private final OnDemandCounter onDemandCounter;
7177

7278
private final long startTime;
7379

@@ -110,6 +116,7 @@ public CrashlyticsCore(
110116
this.backgroundWorker = new CrashlyticsBackgroundWorker(crashHandlerExecutor);
111117

112118
startTime = System.currentTimeMillis();
119+
onDemandCounter = new OnDemandCounter();
113120
}
114121

115122
// endregion
@@ -150,7 +157,8 @@ public boolean onPreExecute(AppData appData, SettingsDataProvider settingsProvid
150157
logFileManager,
151158
userMetadata,
152159
stackTraceTrimmingStrategy,
153-
settingsProvider);
160+
settingsProvider,
161+
onDemandCounter);
154162

155163
controller =
156164
new CrashlyticsController(
@@ -348,6 +356,10 @@ public void setCustomKeys(Map<String, String> keysAndValues) {
348356
controller.setCustomKeys(keysAndValues);
349357
}
350358

359+
// endregion
360+
361+
// region Internal API
362+
351363
/**
352364
* Set a value to be associated with a given key for your crash data. The key/value pairs will be
353365
* reported with any crash that occurs in this session. A maximum of 64 key/value pairs can be
@@ -364,6 +376,19 @@ public void setInternalKey(String key, String value) {
364376
controller.setInternalKey(key, value);
365377
}
366378

379+
/** Logs a fatal Throwable on the Crashlytics servers on-demand. */
380+
public void logFatalException(Throwable throwable) {
381+
Logger.getLogger()
382+
.d("Recorded on-demand fatal events: " + onDemandCounter.getRecordedOnDemandExceptions());
383+
Logger.getLogger()
384+
.d("Dropped on-demand fatal events: " + onDemandCounter.getDroppedOnDemandExceptions());
385+
controller.setInternalKey(
386+
ON_DEMAND_RECORDED_KEY, Integer.toString(onDemandCounter.getRecordedOnDemandExceptions()));
387+
controller.setInternalKey(
388+
ON_DEMAND_DROPPED_KEY, Integer.toString(onDemandCounter.getDroppedOnDemandExceptions()));
389+
controller.logFatalException(Thread.currentThread(), throwable);
390+
}
391+
367392
// endregion
368393

369394
// region Package-protected getters

‎firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/OnDemandCounter.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.crashlytics.internal.common;
16+
17+
import java.util.concurrent.atomic.AtomicInteger;
18+
19+
/** Simple, thread-safe, class to keep count of recorded and dropped on-demand events. */
20+
public final class OnDemandCounter {
21+
private final AtomicInteger recordedOnDemandExceptions;
22+
private final AtomicInteger droppedOnDemandExceptions;
23+
24+
public OnDemandCounter() {
25+
recordedOnDemandExceptions = new AtomicInteger();
26+
droppedOnDemandExceptions = new AtomicInteger();
27+
}
28+
29+
public int getRecordedOnDemandExceptions() {
30+
return recordedOnDemandExceptions.get();
31+
}
32+
33+
public void incrementRecordedOnDemandExceptions() {
34+
recordedOnDemandExceptions.getAndIncrement();
35+
}
36+
37+
public int getDroppedOnDemandExceptions() {
38+
return droppedOnDemandExceptions.get();
39+
}
40+
41+
public void incrementDroppedOnDemandExceptions() {
42+
droppedOnDemandExceptions.getAndIncrement();
43+
}
44+
45+
public void resetDroppedOnDemandExceptions() {
46+
droppedOnDemandExceptions.set(0);
47+
}
48+
}

‎firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,14 @@ public static SessionReportingCoordinator create(
6767
LogFileManager logFileManager,
6868
UserMetadata userMetadata,
6969
StackTraceTrimmingStrategy stackTraceTrimmingStrategy,
70-
SettingsDataProvider settingsProvider) {
70+
SettingsDataProvider settingsProvider,
71+
OnDemandCounter onDemandCounter) {
7172
final CrashlyticsReportDataCapture dataCapture =
7273
new CrashlyticsReportDataCapture(context, idManager, appData, stackTraceTrimmingStrategy);
7374
final CrashlyticsReportPersistence reportPersistence =
7475
new CrashlyticsReportPersistence(fileStore, settingsProvider);
7576
final DataTransportCrashlyticsReportSender reportSender =
76-
DataTransportCrashlyticsReportSender.create(context);
77+
DataTransportCrashlyticsReportSender.create(context, settingsProvider, onDemandCounter);
7778
return new SessionReportingCoordinator(
7879
dataCapture, reportPersistence, reportSender, logFileManager, userMetadata);
7980
}
@@ -202,14 +203,21 @@ public void removeAllReports() {
202203
* sent.
203204
*/
204205
public Task<Void> sendReports(@NonNull Executor reportSendCompleteExecutor) {
206+
return sendReports(reportSendCompleteExecutor, /*sessionId=*/ null);
207+
}
208+
209+
public Task<Void> sendReports(
210+
@NonNull Executor reportSendCompleteExecutor, @Nullable String sessionId) {
205211
final List<CrashlyticsReportWithSessionId> reportsToSend =
206212
reportPersistence.loadFinalizedReports();
207213
final List<Task<Boolean>> sendTasks = new ArrayList<>();
208214
for (CrashlyticsReportWithSessionId reportToSend : reportsToSend) {
209-
sendTasks.add(
210-
reportsSender
211-
.sendReport(reportToSend)
212-
.continueWith(reportSendCompleteExecutor, this::onReportSendComplete));
215+
if (sessionId == null || sessionId.equals(reportToSend.getSessionId())) {
216+
sendTasks.add(
217+
reportsSender
218+
.enqueueReport(reportToSend, sessionId != null)
219+
.continueWith(reportSendCompleteExecutor, this::onReportSendComplete));
220+
}
213221
}
214222
return Tasks.whenAll(sendTasks);
215223
}

‎firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/send/DataTransportCrashlyticsReportSender.java

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,16 @@
1717
import android.content.Context;
1818
import androidx.annotation.NonNull;
1919
import com.google.android.datatransport.Encoding;
20-
import com.google.android.datatransport.Event;
2120
import com.google.android.datatransport.Transformer;
2221
import com.google.android.datatransport.Transport;
2322
import com.google.android.datatransport.cct.CCTDestination;
2423
import com.google.android.datatransport.runtime.TransportRuntime;
2524
import com.google.android.gms.tasks.Task;
26-
import com.google.android.gms.tasks.TaskCompletionSource;
2725
import com.google.firebase.crashlytics.internal.common.CrashlyticsReportWithSessionId;
26+
import com.google.firebase.crashlytics.internal.common.OnDemandCounter;
2827
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport;
2928
import com.google.firebase.crashlytics.internal.model.serialization.CrashlyticsReportJsonTransform;
29+
import com.google.firebase.crashlytics.internal.settings.SettingsDataProvider;
3030
import java.nio.charset.Charset;
3131

3232
/**
@@ -45,10 +45,11 @@ public class DataTransportCrashlyticsReportSender {
4545
private static final Transformer<CrashlyticsReport, byte[]> DEFAULT_TRANSFORM =
4646
(r) -> TRANSFORM.reportToJson(r).getBytes(Charset.forName("UTF-8"));
4747

48-
private final Transport<CrashlyticsReport> transport;
48+
private final ReportQueue reportQueue;
4949
private final Transformer<CrashlyticsReport, byte[]> transportTransform;
5050

51-
public static DataTransportCrashlyticsReportSender create(Context context) {
51+
public static DataTransportCrashlyticsReportSender create(
52+
Context context, SettingsDataProvider settingsProvider, OnDemandCounter onDemandCounter) {
5253
TransportRuntime.initialize(context);
5354
final Transport<CrashlyticsReport> transport =
5455
TransportRuntime.getInstance()
@@ -58,32 +59,21 @@ public static DataTransportCrashlyticsReportSender create(Context context) {
5859
CrashlyticsReport.class,
5960
Encoding.of("json"),
6061
DEFAULT_TRANSFORM);
61-
return new DataTransportCrashlyticsReportSender(transport, DEFAULT_TRANSFORM);
62+
ReportQueue reportQueue =
63+
new ReportQueue(transport, settingsProvider.getSettings(), onDemandCounter);
64+
return new DataTransportCrashlyticsReportSender(reportQueue, DEFAULT_TRANSFORM);
6265
}
6366

6467
DataTransportCrashlyticsReportSender(
65-
Transport<CrashlyticsReport> transport,
66-
Transformer<CrashlyticsReport, byte[]> transportTransform) {
67-
this.transport = transport;
68+
ReportQueue reportQueue, Transformer<CrashlyticsReport, byte[]> transportTransform) {
69+
this.reportQueue = reportQueue;
6870
this.transportTransform = transportTransform;
6971
}
7072

7173
@NonNull
72-
public Task<CrashlyticsReportWithSessionId> sendReport(
73-
@NonNull CrashlyticsReportWithSessionId reportWithSessionId) {
74-
final CrashlyticsReport report = reportWithSessionId.getReport();
75-
76-
TaskCompletionSource<CrashlyticsReportWithSessionId> tcs = new TaskCompletionSource<>();
77-
transport.schedule(
78-
Event.ofUrgent(report),
79-
error -> {
80-
if (error != null) {
81-
tcs.trySetException(error);
82-
return;
83-
}
84-
tcs.trySetResult(reportWithSessionId);
85-
});
86-
return tcs.getTask();
74+
public Task<CrashlyticsReportWithSessionId> enqueueReport(
75+
@NonNull CrashlyticsReportWithSessionId reportWithSessionId, boolean isOnDemand) {
76+
return reportQueue.enqueueReport(reportWithSessionId, isOnDemand).getTask();
8777
}
8878

8979
private static String mergeStrings(String part1, String part2) {

0 commit comments

Comments
 (0)