Skip to content

Commit f693f3d

Browse files
mroberthemiswang
mrober
and
themiswang
authored
Implement Crashlytics Unity ODFs (#549)
* Implement ODFs for Android * Update ReportUncaughtExceptionsAsFatal doc, get, set. * [Crashlytics fatals] Add iOS implementation (#589) * [crashlytics ondemand fatals]fix the fatal logic (#598) * [crashlytics ondemand fatals]fix the fatal logic * remove RecordCustomException and System.Environment.StackTrace frame for fault blame on crashlytics sdk * Fine tune how we handle empty stack traces on Android. Co-authored-by: themiswang <themisw@google.com>
1 parent 6f540ea commit f693f3d

File tree

15 files changed

+183
-9
lines changed

15 files changed

+183
-9
lines changed

‎crashlytics/src/AndroidImpl.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ namespace Firebase.Crashlytics
2020
using System;
2121
using System.Diagnostics;
2222
using System.Collections.Generic;
23+
using System.Linq;
2324

2425
using UnityEngine;
2526

@@ -141,6 +142,39 @@ public override void LogException(Exception exception) {
141142
}, "LogException");
142143
}
143144

145+
public override void LogExceptionAsFatal(Exception exception) {
146+
var loggedException = LoggedException.FromException(exception);
147+
Dictionary<string, string>[] parsedStackTrace = loggedException.ParsedStackTrace;
148+
149+
if (parsedStackTrace.Length == 0) {
150+
// if for some reason we don't get stack trace from exception, we add current stack trace in
151+
var currentStackTrace = System.Environment.StackTrace;
152+
LoggedException loggedExceptionWithCurrentStackTrace = new LoggedException(loggedException.Name, loggedException.Message, currentStackTrace);
153+
parsedStackTrace = loggedExceptionWithCurrentStackTrace.ParsedStackTrace;
154+
155+
if (parsedStackTrace.Length > 3) {
156+
// remove AndroidImpl frames for fault blame on crashlytics sdk
157+
var slicedParsedStackTrace = parsedStackTrace.Skip(3).Take(parsedStackTrace.Length - 3).ToArray();
158+
parsedStackTrace = slicedParsedStackTrace;
159+
}
160+
}
161+
162+
StackFrames frames = new StackFrames();
163+
foreach (Dictionary<string, string> frame in parsedStackTrace) {
164+
frames.Add(new FirebaseCrashlyticsFrame {
165+
library = frame["class"],
166+
symbol = frame["method"],
167+
fileName = frame["file"],
168+
lineNumber = frame["line"],
169+
});
170+
}
171+
172+
CallInternalMethod(() => {
173+
crashlyticsInternal.LogExceptionAsFatal(loggedException.Name,
174+
loggedException.Message, frames);
175+
}, "LogExceptionAsFatal");
176+
}
177+
144178
public override bool IsCrashlyticsCollectionEnabled() {
145179
return CallInternalMethod(() => {
146180
return crashlyticsInternal.IsCrashlyticsCollectionEnabled();

‎crashlytics/src/Crashlytics.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@ namespace Firebase.Crashlytics {
2828
[UnityEngine.Scripting.Preserve]
2929
public static class Crashlytics {
3030

31+
/// <summary>
32+
/// Whether Crashlytics is set to report uncaught exceptions as fatal.
33+
/// Fatal exceptions count towards Crash Free Users and Velocity Alerts.
34+
/// It is recommended to enable this for new apps.
35+
/// <returns>
36+
/// true if Crashlytics is set to report uncaught exceptions as fatal, false otherwise.
37+
/// </returns>
38+
/// </summary>
39+
public static bool ReportUncaughtExceptionsAsFatal { get; set; } = false;
40+
3141
/// <summary>
3242
/// Checks whether the Crashlytics specific data collection flag has been enabled.
3343
/// <returns>
@@ -106,6 +116,16 @@ public static void LogException(Exception exception) {
106116
PlatformAccessor.Impl.LogException(exception);
107117
}
108118

119+
/// <summary>
120+
/// Record a fatal exception.
121+
/// </summary>
122+
/// <param name="exception">
123+
/// The exception to log as fatal.
124+
/// </param>
125+
public static void LogExceptionAsFatal(Exception exception) {
126+
PlatformAccessor.Impl.LogExceptionAsFatal(exception);
127+
}
128+
109129
/// <summary>
110130
/// This class holds a privately held instances that should be accessed via
111131
/// the internal getters. This allows us to lazily initialize the instances

‎crashlytics/src/ExceptionHandler.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ internal virtual void LogException(LoggedException e) {
9494
"Exception stack trace:\n" +
9595
"{1}", e.Message, e.StackTrace)
9696
);
97-
Crashlytics.LogException(e);
97+
if (Crashlytics.ReportUncaughtExceptionsAsFatal) {
98+
Crashlytics.LogExceptionAsFatal(e);
99+
} else {
100+
Crashlytics.LogException(e);
101+
}
98102
}
99103
}
100104
}

‎crashlytics/src/IOSImpl.cs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ namespace Firebase.Crashlytics
2020
using System.Runtime.InteropServices;
2121
using System.Diagnostics;
2222
using System.Collections.Generic;
23+
using System.Linq;
2324
using UnityEngine;
2425

2526

@@ -60,7 +61,7 @@ internal class IOSImpl : Impl
6061

6162
[DllImport("__Internal")]
6263
private static extern void CLURecordCustomException(string name, string reason,
63-
Frame[] frames, int frameCount);
64+
Frame[] frames, int frameCount, bool isOnDemand);
6465

6566
[DllImport("__Internal")]
6667
private static extern bool CLUIsCrashlyticsCollectionEnabled();
@@ -96,7 +97,12 @@ public override void SetUserId(string identifier)
9697
public override void LogException(Exception exception)
9798
{
9899
var loggedException = LoggedException.FromException(exception);
99-
RecordCustomException(loggedException);
100+
RecordCustomException(loggedException, false);
101+
}
102+
103+
public override void LogExceptionAsFatal(Exception exception) {
104+
var loggedException = LoggedException.FromException(exception);
105+
RecordCustomException(loggedException, true);
100106
}
101107

102108
public override bool IsCrashlyticsCollectionEnabled() {
@@ -108,9 +114,22 @@ public override void SetCrashlyticsCollectionEnabled(bool enabled) {
108114
}
109115

110116
// private void RecordCustomException(string name, string reason, string stackTraceString)
111-
private void RecordCustomException(LoggedException loggedException) {
117+
private void RecordCustomException(LoggedException loggedException, bool isOnDemand) {
112118
Dictionary<string, string>[] parsedStackTrace = loggedException.ParsedStackTrace;
113119

120+
if (isOnDemand && parsedStackTrace.Length == 0) {
121+
// if for some reason we don't get stack trace from exception, we add current stack trace in
122+
var currentStackTrace = System.Environment.StackTrace;
123+
LoggedException loggedExceptionWithCurrentStackTrace = new LoggedException(loggedException.Name, loggedException.Message, currentStackTrace);
124+
parsedStackTrace = loggedExceptionWithCurrentStackTrace.ParsedStackTrace;
125+
126+
if (parsedStackTrace.Length > 2) {
127+
// remove RecordCustomException and System.Environment.StackTrace frame for fault blame on crashlytics sdk
128+
var slicedParsedStackTrace = parsedStackTrace.Skip(2).Take(parsedStackTrace.Length - 2).ToArray();
129+
parsedStackTrace = slicedParsedStackTrace;
130+
}
131+
}
132+
114133
List<Frame> frames = new List<Frame>();
115134
foreach (Dictionary<string, string> frame in parsedStackTrace) {
116135
frames.Add(new Frame {
@@ -121,7 +140,7 @@ private void RecordCustomException(LoggedException loggedException) {
121140
});
122141
}
123142

124-
CLURecordCustomException(loggedException.Name, loggedException.Message, frames.ToArray(), frames.Count);
143+
CLURecordCustomException(loggedException.Name, loggedException.Message, frames.ToArray(), frames.Count, isOnDemand);
125144
}
126145
}
127146
}

‎crashlytics/src/Impl.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ internal class Impl {
3535
"Would set user identifier if running on a physical device: {0}";
3636
private static readonly string LogExceptionString =
3737
"Would log exception if running on a physical device: {0}";
38+
private static readonly string LogExceptionAsFatalString =
39+
"Would log exception as fatal if running on a physical device: {0}";
3840
private static readonly string IsCrashlyticsCollectionEnabledString =
3941
"Would get Crashlytics data collection if running on a physical device";
4042
private static readonly string SetCrashlyticsCollectionEnabledString =
@@ -70,6 +72,10 @@ public virtual void LogException(Exception exception) {
7072
LogUtil.LogMessage(LogLevel.Debug, String.Format(LogExceptionString, exception));
7173
}
7274

75+
public virtual void LogExceptionAsFatal(Exception exception) {
76+
LogUtil.LogMessage(LogLevel.Debug, String.Format(LogExceptionAsFatalString, exception));
77+
}
78+
7379
public virtual bool IsCrashlyticsCollectionEnabled() {
7480
LogUtil.LogMessage(LogLevel.Debug, String.Format(IsCrashlyticsCollectionEnabledString));
7581
return true;

‎crashlytics/src/cpp/android/crashlytics_android.cc‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,25 @@ METHOD_LOOKUP_DEFINITION(firebase_crashlytics,
6262
CRASHLYTICS_METHODS, CRASHLYTICS_FIELDS)
6363

6464
// clang-format off
65+
#define CRASHLYTICS_CORE_METHODS(X) \
66+
X(LogFatalException, "logFatalException", \
67+
"(Ljava/lang/Throwable;)V", \
68+
util::kMethodTypeInstance)
69+
6570
#define CRASHLYTICS_CORE_FIELDS(X) \
6671
X(DataCollectionArbiter, "dataCollectionArbiter", \
6772
"Lcom/google/firebase/crashlytics/internal/common/DataCollectionArbiter;", \
6873
util::kFieldTypeInstance)
6974

7075
// clang-format on
7176
METHOD_LOOKUP_DECLARATION(crashlytics_core,
72-
METHOD_LOOKUP_NONE, CRASHLYTICS_CORE_FIELDS)
77+
CRASHLYTICS_CORE_METHODS,
78+
CRASHLYTICS_CORE_FIELDS)
7379
METHOD_LOOKUP_DEFINITION(
7480
crashlytics_core,
7581
PROGUARD_KEEP_CLASS
7682
"com/google/firebase/crashlytics/internal/common/CrashlyticsCore",
77-
METHOD_LOOKUP_NONE, CRASHLYTICS_CORE_FIELDS)
83+
CRASHLYTICS_CORE_METHODS, CRASHLYTICS_CORE_FIELDS)
7884

7985
// clang-format off
8086
#define CRASHLYTICS_DATA_COLLECTION_METHODS(X) \
@@ -144,6 +150,7 @@ bool cached_data_collection_enabled_ = false;
144150

145151
CrashlyticsInternal::CrashlyticsInternal(App* app) {
146152
data_collection_obj_ = nullptr;
153+
core_ = nullptr;
147154
obj_ = nullptr;
148155
java_vm_ = app->java_vm();
149156

@@ -188,6 +195,7 @@ CrashlyticsInternal::CrashlyticsInternal(App* app) {
188195
env->DeleteLocalRef(application_context);
189196
assert(data_collection_obj != nullptr);
190197
data_collection_obj_ = env->NewGlobalRef(data_collection_obj);
198+
core_ = env->NewGlobalRef(core);
191199
env->DeleteLocalRef(data_collection_obj);
192200
env->DeleteLocalRef(core);
193201

@@ -212,6 +220,10 @@ CrashlyticsInternal::~CrashlyticsInternal() {
212220
env->DeleteGlobalRef(data_collection_obj_);
213221
data_collection_obj_ = nullptr;
214222
}
223+
if (core_) {
224+
env->DeleteGlobalRef(core_);
225+
core_ = nullptr;
226+
}
215227
Terminate();
216228
java_vm_ = nullptr;
217229

@@ -227,6 +239,7 @@ bool CrashlyticsInternal::Initialize(JNIEnv* env, jobject activity) {
227239
if (!(firebase_crashlytics::CacheMethodIds(env, activity) &&
228240
firebase_crashlytics::CacheFieldIds(env, activity) &&
229241
firebase_crashlytics_ndk::CacheMethodIds(env, activity) &&
242+
crashlytics_core::CacheMethodIds(env, activity) &&
230243
crashlytics_core::CacheFieldIds(env, activity) &&
231244
crashlytics_data_collection::CacheMethodIds(env, activity) &&
232245
java_exception::CacheMethodIds(env, activity) &&
@@ -340,6 +353,29 @@ void CrashlyticsInternal::LogException(
340353
env->DeleteLocalRef(exception_object);
341354
}
342355

356+
void CrashlyticsInternal::LogExceptionAsFatal(
357+
const char* name, const char* reason,
358+
std::vector<firebase::crashlytics::Frame> frames) {
359+
if (!cached_data_collection_enabled_) {
360+
return;
361+
}
362+
JNIEnv* env = util::GetThreadsafeJNIEnv(java_vm_);
363+
364+
std::string message(name);
365+
message += EXCEPTION_MESSAGE_SEPARATOR;
366+
message += reason;
367+
368+
jobject exception_object = BuildJavaException(message, frames);
369+
370+
env->CallVoidMethod(
371+
core_,
372+
crashlytics_core::GetMethodId(crashlytics_core::kLogFatalException),
373+
exception_object);
374+
util::LogException(env, kLogLevelError,
375+
"Crashlytics::LogExceptionAsFatal() failed");
376+
env->DeleteLocalRef(exception_object);
377+
}
378+
343379
bool CrashlyticsInternal::GetCrashlyticsCollectionEnabled(
344380
JavaVM* java_vm, jobject data_collection_obj) {
345381
JNIEnv* env = util::GetThreadsafeJNIEnv(java_vm);

‎crashlytics/src/cpp/android/crashlytics_android.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ class CrashlyticsInternal {
4545
void SetUserId(const char* id);
4646
void LogException(const char* name, const char* reason,
4747
std::vector<firebase::crashlytics::Frame> frames);
48+
void LogExceptionAsFatal(const char* name, const char* reason,
49+
std::vector<firebase::crashlytics::Frame> frames);
4850
bool IsCrashlyticsCollectionEnabled();
4951
void SetCrashlyticsCollectionEnabled(bool enabled);
5052

@@ -72,6 +74,9 @@ class CrashlyticsInternal {
7274

7375
// Java DataCollectionArbiter global ref.
7476
jobject data_collection_obj_;
77+
78+
// Java CrashlyticsCore global ref.
79+
jobject core_;
7580
};
7681

7782
} // namespace internal

‎crashlytics/src/cpp/common/crashlytics.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ void Crashlytics::LogException(const char* name, const char* reason,
9191
internal_->LogException(name, reason, frames);
9292
}
9393

94+
void Crashlytics::LogExceptionAsFatal(const char* name, const char* reason,
95+
std::vector<Frame> frames) {
96+
internal_->LogExceptionAsFatal(name, reason, frames);
97+
}
98+
9499
bool Crashlytics::IsCrashlyticsCollectionEnabled() {
95100
return internal_->IsCrashlyticsCollectionEnabled();
96101
}

‎crashlytics/src/cpp/include/firebase/crashlytics.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ class Crashlytics {
6969
void SetUserId(const char* id);
7070
void LogException(const char* name, const char* reason,
7171
std::vector<Frame> frames);
72+
void LogExceptionAsFatal(const char* name, const char* reason,
73+
std::vector<Frame> frames);
7274
bool IsCrashlyticsCollectionEnabled();
7375
void SetCrashlyticsCollectionEnabled(bool enabled);
7476

‎crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/Crashlytics_Platform.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
@property(nonatomic, strong, nullable) NSString* developmentPlatformName;
2626
@property(nonatomic, strong, nullable) NSString* developmentPlatformVersion;
2727

28+
- (void)recordOnDemandExceptionModel:(FIRExceptionModel* _Nonnull)exceptionModel;
2829
@end
2930

3031
void FIRCLSUserLoggingRecordInternalKeyValue(NSString* key, id value);

0 commit comments

Comments
 (0)