From 9e3c833ac654fb627be8c0eff7f83d85e6cd1bea Mon Sep 17 00:00:00 2001 From: Themis wang Date: Thu, 12 Jan 2023 17:26:50 -0500 Subject: [PATCH 1/2] [crashlytics ondemand fatals]fix the fatal logic --- crashlytics/src/AndroidImpl.cs | 11 +++++++- crashlytics/src/IOSImpl.cs | 7 +++++ .../ExceptionModel_Platform.h | 28 +++++++++++++++++++ .../src/cpp/ios/CrashlyticsiOSWrapper.m | 4 +++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h diff --git a/crashlytics/src/AndroidImpl.cs b/crashlytics/src/AndroidImpl.cs index fc65153b..1db52548 100644 --- a/crashlytics/src/AndroidImpl.cs +++ b/crashlytics/src/AndroidImpl.cs @@ -143,6 +143,15 @@ public override void LogException(Exception exception) { public override void LogExceptionAsFatal(Exception exception) { var loggedException = LoggedException.FromException(exception); + Dictionary[] parsedStackTrace = loggedException.ParsedStackTrace; + + if (parsedStackTrace.Length == 0) { + // if for some reason we don't get stack trace from exception, we add current stack trace in + var currentStackTrace = System.Environment.StackTrace; + LoggedException loggedExceptionWithCurrentStackTrace = new LoggedException(loggedException.Name, loggedException.Message, currentStackTrace); + parsedStackTrace = loggedExceptionWithCurrentStackTrace.ParsedStackTrace; + } + StackFrames frames = new StackFrames(); foreach (Dictionary frame in loggedException.ParsedStackTrace) { frames.Add(new FirebaseCrashlyticsFrame { @@ -152,7 +161,7 @@ public override void LogExceptionAsFatal(Exception exception) { lineNumber = frame["line"], }); } - // TODO(mrober): Do something for empty stacktrace. Get current stacktrace? + CallInternalMethod(() => { crashlyticsInternal.LogExceptionAsFatal(loggedException.Name, loggedException.Message, frames); diff --git a/crashlytics/src/IOSImpl.cs b/crashlytics/src/IOSImpl.cs index 05007d5a..7ad049dc 100644 --- a/crashlytics/src/IOSImpl.cs +++ b/crashlytics/src/IOSImpl.cs @@ -116,6 +116,13 @@ public override void SetCrashlyticsCollectionEnabled(bool enabled) { private void RecordCustomException(LoggedException loggedException, bool isOnDemand) { Dictionary[] parsedStackTrace = loggedException.ParsedStackTrace; + if (isOnDemand && parsedStackTrace.Length == 0) { + // if for some reason we don't get stack trace from exception, we add current stack trace in + var currentStackTrace = System.Environment.StackTrace; + LoggedException loggedExceptionWithCurrentStackTrace = new LoggedException(loggedException.Name, loggedException.Message, currentStackTrace); + parsedStackTrace = loggedExceptionWithCurrentStackTrace.ParsedStackTrace; + } + List frames = new List(); foreach (Dictionary frame in parsedStackTrace) { frames.Add(new Frame { diff --git a/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h b/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h new file mode 100644 index 00000000..a750a8d2 --- /dev/null +++ b/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h @@ -0,0 +1,28 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +// +// Crashlytics_ExceptionModel.h +// Crashlytics +// + +#import + +@interface FIRExceptionModel (Platform) + +@property(nonatomic) BOOL isFatal; +@property(nonatomic) BOOL onDemand; + +@end \ No newline at end of file diff --git a/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m b/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m index ff5ca2f3..59790096 100644 --- a/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m +++ b/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m @@ -21,6 +21,7 @@ #import "CrashlyticsiOSWrapper.h" #import "FirebaseCrashlytics.h" #import "./Crashlytics_PrivateHeaders/Crashlytics_Platform.h" +#import "./Crashlytics_PrivateHeaders/ExceptionModel_Platform.h" @interface FIRCrashlytics () - (BOOL)isCrashlyticsStarted; @@ -58,6 +59,9 @@ void CLURecordCustomException(const char *name, const char *reason, Frame *frame model.stackTrace = framesArray; if (isOnDemand) { + // For on demand exception, we log them as fatal + model.onDemand = isOnDemand; + model.isFatal = fatal; [[FIRCrashlytics crashlytics] recordOnDemandExceptionModel:model]; return; } From 5e200f649eacb58a10c17d731cc81500f13da616 Mon Sep 17 00:00:00 2001 From: Themis wang Date: Fri, 13 Jan 2023 00:26:27 -0500 Subject: [PATCH 2/2] remove RecordCustomException and System.Environment.StackTrace frame for fault blame on crashlytics sdk --- crashlytics/src/AndroidImpl.cs | 7 +++++++ crashlytics/src/IOSImpl.cs | 7 +++++++ .../Crashlytics_PrivateHeaders/ExceptionModel_Platform.h | 2 +- crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m | 4 ++-- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crashlytics/src/AndroidImpl.cs b/crashlytics/src/AndroidImpl.cs index 1db52548..4ed92c00 100644 --- a/crashlytics/src/AndroidImpl.cs +++ b/crashlytics/src/AndroidImpl.cs @@ -20,6 +20,7 @@ namespace Firebase.Crashlytics using System; using System.Diagnostics; using System.Collections.Generic; + using System.Linq; using UnityEngine; @@ -150,6 +151,12 @@ public override void LogExceptionAsFatal(Exception exception) { var currentStackTrace = System.Environment.StackTrace; LoggedException loggedExceptionWithCurrentStackTrace = new LoggedException(loggedException.Name, loggedException.Message, currentStackTrace); parsedStackTrace = loggedExceptionWithCurrentStackTrace.ParsedStackTrace; + + if (parsedStackTrace.Length > 2) { + // remove RecordCustomException and System.Environment.StackTrace frame for fault blame on crashlytics sdk + var slicedParsedStackTrace = parsedStackTrace.Skip(2).Take(parsedStackTrace.Length - 2).ToArray(); + parsedStackTrace = slicedParsedStackTrace; + } } StackFrames frames = new StackFrames(); diff --git a/crashlytics/src/IOSImpl.cs b/crashlytics/src/IOSImpl.cs index 7ad049dc..5793fc6b 100644 --- a/crashlytics/src/IOSImpl.cs +++ b/crashlytics/src/IOSImpl.cs @@ -20,6 +20,7 @@ namespace Firebase.Crashlytics using System.Runtime.InteropServices; using System.Diagnostics; using System.Collections.Generic; + using System.Linq; using UnityEngine; @@ -121,6 +122,12 @@ private void RecordCustomException(LoggedException loggedException, bool isOnDem var currentStackTrace = System.Environment.StackTrace; LoggedException loggedExceptionWithCurrentStackTrace = new LoggedException(loggedException.Name, loggedException.Message, currentStackTrace); parsedStackTrace = loggedExceptionWithCurrentStackTrace.ParsedStackTrace; + + if (parsedStackTrace.Length > 2) { + // remove RecordCustomException and System.Environment.StackTrace frame for fault blame on crashlytics sdk + var slicedParsedStackTrace = parsedStackTrace.Skip(2).Take(parsedStackTrace.Length - 2).ToArray(); + parsedStackTrace = slicedParsedStackTrace; + } } List frames = new List(); diff --git a/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h b/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h index a750a8d2..605b0db5 100644 --- a/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h +++ b/crashlytics/src/cpp/ios/Crashlytics_PrivateHeaders/ExceptionModel_Platform.h @@ -1,5 +1,5 @@ /* - * Copyright 2018 Google LLC + * Copyright 2023 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m b/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m index 59790096..4ad645d1 100644 --- a/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m +++ b/crashlytics/src/cpp/ios/CrashlyticsiOSWrapper.m @@ -60,8 +60,8 @@ void CLURecordCustomException(const char *name, const char *reason, Frame *frame if (isOnDemand) { // For on demand exception, we log them as fatal - model.onDemand = isOnDemand; - model.isFatal = fatal; + model.onDemand = YES; + model.isFatal = YES; [[FIRCrashlytics crashlytics] recordOnDemandExceptionModel:model]; return; }