hansemannn / titanium-crashlytics

Use the native Crashlytics SDK in Titanium (iOS / Android).
MIT License
18 stars 18 forks source link

Ti SDK 9.3.2.GA + ti.crashlytics 2.1.0 Android = NullPointerException on start up #38

Closed phillipmovista closed 3 years ago

phillipmovista commented 3 years ago

Thank you all so much for these modules!

I am working on getting ti.crashlytics 2.1.0 into my Android app. I used the basic require('ti.crashlytics'); in the code and I do see it going to Crashlytics in Firebase, but oddly enough, it immediately fires off a crash of:

Non-fatal Exception: java.lang.Exception

Non-fatal Exception: java.lang.Exception
       at ti.crashlytics.TitaniumCrashlyticsModule.onAppCreate(TitaniumCrashlyticsModule.java:50)

This seems pretty odd since it got to Firebase okay, but then it seems to complain that something in this line is not playing well:

FirebaseCrashlytics.getInstance().recordException(new Exception());

Thanks so much!

macasfaj commented 3 years ago

I have the same issue. I was taking about it with @m1ga recently 😞

m1ga commented 3 years ago

@phillipmovista that line was recommended as a workaround by users in the firbase native users and it should trigger a push to upload all stored exceptions. But the problem is that it is the only exception we are seeing now. Other exceptions are not logged or seen.

Looks like it is not a Titanium only issue: https://github.com/firebase/firebase-android-sdk/issues/1944#issuecomment-883630065 but hopefully we can find a way around it.

There is a Sentry module https://github.com/AppWerft/Ti.Sentry which might be worth updating

phillipmovista commented 3 years ago

I was looking at it and wondering what caused the NullPointerException. Maybe FirebaseCrashlytics.getInstance() is null at the time that is called or something?

We already have newrelic in place but we are having to jump off it since I suppose they do not support Android Ti SDK 9

m1ga commented 3 years ago

its new Exception() inside recordException :wink: That throws the exception and triggers the connection to the server. But it should upload all other exceptions too which is not happening

phillipmovista commented 3 years ago

Oh, super weird. I have this running first thing in my app (to load up crashlytics) maybe that has something to do with it? Possible that it needs to check for if any exceptions exist to upload and then send them?

m1ga commented 3 years ago

normally you don't need that and it should upload logs by its own. But it stopped working so we had a look and found that issue above where other (native) people have the same issue. But again: the exception is intended but it should upload more which it doesn't. We've tried other workarounds but it all ended up in just seeing that crashlog and no other logs.

Have to do some more tests and check if there is any workaround or if we have to wait for the next version of crashlytics

phillipmovista commented 3 years ago

Sound good, thank you. I will hopefully see an updated version as well.

m1ga commented 3 years ago

You can try this version: https://migaweb.de/ti.crashlytics-android-2.1.1.zip where I removed the FirebaseCrashlytics.getInstance().recordException(new Exception()); So it is the "old" version with the latest dependencies. There was no log at all in our tests, perhaps you'll have more luck with it

phillipmovista commented 3 years ago

2.1.1 is acting the same for me - thanks for trying.

m1ga commented 3 years ago

wait, with the init exception again or just no results?

phillipmovista commented 3 years ago

init exception. still same error as above.

m1ga commented 3 years ago

that code shouldn't be in there :thinking: Try this one https://migaweb.de/ti.crashlytics-android-2.1.1b.zip perhaps I've uploaded the wrong file

phillipmovista commented 3 years ago

Initial tests show this is better. It did not kick out the error message at first...but I also have no way to tell if it is actually working when I have a real crash. We will keep testing.

m1ga commented 3 years ago

@phillipmovista you could run .crash() from the module to force a crash. Would be good to know if that is working for you

phillipmovista commented 3 years ago

It does show the program crashes but then I do not see anything showing up in crashlytics after that time.

m1ga commented 3 years ago

ok, that was the initial "bug": crashlogs won't be uploaded. There was another person in the firebase repo that repots the same behavior in a native app. So it looks like its nothing we can do at the moment.

phillipmovista commented 3 years ago

Darn, well, thank you for letting me know. I will share this with my team since we have other apps that may be wanting this same functionality and it looks like there could be other issues with crashlytics in mobile apps.

phillipmovista commented 3 years ago

@m1ga any chance you can link me to that other firebase repo discussion?

Also, is there anything we can do to have the 2.1.1 code to see the steps that are being taken on this?

m1ga commented 3 years ago

link is mentioned here https://github.com/hansemannn/titanium-crashlytics/issues/38#issuecomment-922049998

The whole module code is just those lines: https://github.com/hansemannn/titanium-crashlytics/blob/master/android/src/ti/crashlytics/TitaniumCrashlyticsModule.java#L24-L51 and I just removed the line FirebaseCrashlytics.getInstance().recordException(new Exception()); again

phillipmovista commented 3 years ago

Thank you, I had overlooked that by mistake. We will keep digging from there.

petenathan42 commented 3 years ago

crashlytics 18.2.3 is out: https://mvnrepository.com/artifact/com.google.firebase/firebase-crashlytics?repo=google

along with analytics 19.0.2 https://mvnrepository.com/artifact/com.google.firebase/firebase-analytics?repo=google

If you want to give it a shot. Still not a lot of luck from my end, but wanted to share!

m1ga commented 3 years ago

Thanks for the info @petenathan42 nothing in my logs either and I don't see any release logs of 18.2.3

petenathan42 commented 3 years ago

I can't really tell if this is a crashlytics issue or something funky going on with how it's integrated with titanium. You would expect an event to be created with a fatal test crash, but there's absolute silence in the firebase logs

long explanation posted in the bigger thread:

I am getting the: 10-06 17:03:17.538 19155 19193 V FirebaseCrashlytics: Persisting non-fatal event for session 615E1CC3033500014AD34B023E324821 log when running: FirebaseCrashlytics.getInstance().recordException(new RuntimeException("test crash")); In my test crash function.

the "Persisting non-fatal event for session" occurrence, on android, creates an event file in: /data/user/0/<app-name>/files/.com.google.firebase.crashlytics/report-persistence/sessions/<session>/ the file name in my case was "event0000000000"

So after you restart the application, a report is created based on that event and then that report is sent to the firebase console.

However,

When I run my test crash function in the same place, but instead with a fatal crash: throw new RuntimeException("test crash"); I just get silence in the log. Why isn't an event being created in this situation? An event is never created, so a report is never created, and so I never get anything in the firebase console.

I'm using Crashlytics 18.2.1 (also tried 18.2.3) Analytics 19.0.1 (also tried 19.0.2) Firebase Crashlytics Gradle 2.7.1

FYI, to get the verbose logs, run the command: adb shell setprop log.tag.FirebaseCrashlytics VERBOSE Then, run the adb logcat -s FirebaseCrashlytics TransportRuntime.SQLiteEventStore TransportRuntime.JobInfoScheduler TransportRuntime.CctTransportBackend TransportRuntime.DefaultScheduler

to see the file system, run adb shell as root, and navigate to /data/user/0/<app name>/files/.com.google.firebase.crashlytics/report-persistence/

m1ga commented 3 years ago

as you can see in the native thread where you've posted too: there are native people having the same issues and there was team member post that they can reproduce the error (20 Jul) but that workaround didn't really work. It will just send a new exception but not the log you'll want to have. So I don't know any solution at the moment

petenathan42 commented 3 years ago

I'm not sure if the issue I am having is the same one as in that native thread. It seems like they are getting events written in device storage, but the sending is failing. My problem is that a crash event isn't ever being written to the device's file system.

In the example project reference by the 20 Jul comment (Jan 27), he's saying the report is failing to send depending on how late the crash occurs in the app.

In my example, I'm never getting any crash data written, no matter how late in the app I run the test crash.

I would be interested to know what happens in your adb shell setprop log.tag.FirebaseCrashlytics VERBOSE log when running a test crash. Any sign of life in the log at the point of the crash would already be better than what I'm getting.

petenathan42 commented 3 years ago

I've figured out what's going on.

Titanium's setDefaultUncaughtExceptionHandler is competing with the crashlytics setDefaultUncaughtExceptionHandler

There can only be one default uncaught exception handler.

this is the closest thing to a solution I have found.

I wrote:

image

which injects crashlytics' recordException function into titanium's exception handler. I am now getting -somewhat- useful logs in the firebase console (it works with the test crash function and real crashes in the app), but it would be really great if there was a way to use crashlytics to its full potential. Meaning, the crashlytics default uncaught exception handler runs in parallel with titanium's.

By somewhat useful, I mean the only thing that ends up going to the firebase console is "error.message" It would be a lot better if I had access to the entire throwable error (e), so I could just pass that into FirebaseCrashlytics.getInstance().recordException but I'm not seeing a way to get the error through https://docs.appcelerator.com/module-apidoc/latest/android/org/appcelerator/kroll/KrollRuntime.html

Any suggestions?

m1ga commented 3 years ago

@petenathan42 nice progress, going to have a look at that. The interesting part is only: it used to work in previous versions and the Ti exception part is there since a long time https://github.com/appcelerator/titanium_mobile/pull/10003 (the main function since the beginning).

This issued started with FirebaseCrashlytics.getInstance().recordException(new Exception());where we just had an "empty" exception pushed to the firebase console. So we had some log but it was just an empty exception. I'll check the logs later on and test your method

m1ga commented 3 years ago

try

    @Kroll.onAppCreate
    public static void onAppCreate(TiApplication app) {
        KrollRuntime.setPrimaryExceptionHandler(new TiExceptionHandler(){
            @Override
            public void handleException(ExceptionMessage error) {
                FirebaseCrashlytics.getInstance().recordException(new Exception(error.javaStack));
                super.handleException(error);
            }
        });
    }

(please post code and not a screenshot next time :wink: Makes it easier to copy)

That prints some more infos to the crashlog: Screenshot_20211008_123352

m1ga commented 3 years ago

@phillipmovista if you want to give it a try: https://migaweb.de/ti.crashlytics-android-2.1.2.zip

for the test crash you'll see

Non-fatal Exception: java.lang.Exception: Uncaught Error: This is a crash

    ti.crashlytics.TitaniumCrashlyticsModule.crash(TitaniumCrashlyticsModule.java:38)
    org.appcelerator.kroll.runtime.v8.V8Object.nativeFireEvent(Native Method)
    org.appcelerator.kroll.runtime.v8.V8Object.fireEvent(V8Object.java:63)
    org.appcelerator.kroll.KrollProxy.doFireEvent(KrollProxy.java:985)
    org.appcelerator.kroll.KrollProxy.handleMessage(KrollProxy.java:1219)
    org.appcelerator.titanium.proxy.TiViewProxy.handleMessage(TiViewProxy.java:260)
    android.os.Handler.dispatchMessage(Handler.java:102)
    android.os.Looper.loop(Looper.java:223)
    android.app.ActivityThread.main(ActivityThread.java:7664)
    java.lang.reflect.Method.invoke(Native Method)

       at ti.crashlytics.TitaniumCrashlyticsModule$1.handleException(TitaniumCrashlyticsModule.java:58)
       at org.appcelerator.kroll.KrollRuntime.dispatchException(KrollRuntime.java:541)
       at org.appcelerator.kroll.runtime.v8.V8Object.nativeFireEvent(V8Object.java)
       at org.appcelerator.kroll.runtime.v8.V8Object.fireEvent(V8Object.java:63)
       at org.appcelerator.kroll.KrollProxy.doFireEvent(KrollProxy.java:985)
       at org.appcelerator.kroll.KrollProxy.handleMessage(KrollProxy.java:1219)
       at org.appcelerator.titanium.proxy.TiViewProxy.handleMessage(TiViewProxy.java:260)
       at android.os.Handler.dispatchMessage(Handler.java:102)
       at android.os.Looper.loop(Looper.java:223)
       at android.app.ActivityThread.main(ActivityThread.java:7664)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

Message, Stacktrace and the where the event was fired in the module (should always be the same and not useful :) )

m1ga commented 3 years ago

Screenshot_20211008_125816

Throwable th = new Throwable();
StackTraceElement[] trace = new StackTraceElement[] {
    new StackTraceElement(error.sourceName, error.message, error.lineSource, error.line)
};
th.setStackTrace(trace);
FirebaseCrashlytics.getInstance().recordException(th);

:thinking: that could even be better

phillipmovista commented 3 years ago

@m1ga thanks so much!! Lucky for me... @petenathan42 is working with me on this one, so I will let him pick up your suggestion today and see how it goes.

🤞

petenathan42 commented 3 years ago
Throwable th = new Throwable();
StackTraceElement[] trace = new StackTraceElement[] {
  new StackTraceElement(error.sourceName, error.message, error.lineSource, error.line)
};
th.setStackTrace(trace);
FirebaseCrashlytics.getInstance().recordException(th);

Just now getting back into this, I was going to try this! I thought "maybe I could build a Throwable based on the info in "error", but you beat me to it!

petenathan42 commented 3 years ago

I expanded on that idea a bit. I have formatted the stack trace that crashlystics consumes with titanium's error.javaStack and error.jsStack in mind. The top-level error is still new StackTraceElement(error.sourceName, error.message, error.lineSource, error.line);, but now the stack trace also includes error.javaStack and error.jsStack

I noticed that sometimes error.javaStack is null, so I just took care of all 4 cases.

both error.javaStack and error.jsStack exist: image

only error.javaStack exists: image

only error.jsStack exists (this was a real crash): image

no stack available: image

module code:

    public static StackTraceElement[] generateStackTrace(String javaStack, String jsStack, String errorSourceName, String errorMessage, String errorLineSource, int errorLine) {
        String javaHeader = "------------------------------JAVA STACK.TRACE------------------------------(JAVA STACK.TRACE:0)" + System.lineSeparator();
        String jsHeader = "------------------------JAVACRIPT STACK.TRACE------------------------(JAVASCRIPT STACK.TRACE:0)" + System.lineSeparator();
        String bigStack;

        if (javaStack != null && jsStack != null) {
            bigStack = javaHeader + javaStack.substring(javaStack.indexOf(System.lineSeparator()) + 1) + jsHeader + jsStack.substring(jsStack.indexOf(System.lineSeparator()) + 1);
        } else if (javaStack != null) {
            bigStack = javaHeader + javaStack.substring(javaStack.indexOf(System.lineSeparator()) + 1);
        } else if (jsStack != null) {
            bigStack = jsHeader + jsStack.substring(jsStack.indexOf(System.lineSeparator()) + 1);
        } else {
            StackTraceElement[] trace = new StackTraceElement[] {
                new StackTraceElement(errorSourceName, errorMessage, errorLineSource, errorLine)
            };
            return trace;
        }

        String[] stackElements = bigStack.split(System.lineSeparator());
        StackTraceElement[] trace = new StackTraceElement[stackElements.length + 1];
        trace[0] = new StackTraceElement(errorSourceName, errorMessage, errorLineSource, errorLine);
        for (int i = 0; i < stackElements.length; i++) {
            Log.i(TAG, "javaStackElements " + i + ": " + stackElements[i]);
            String[] splitByParen = stackElements[i].substring(0, stackElements[i].length() - 1).split("\\(");

            String fileName = "";
            int lineNumber = 0;
            if (splitByParen[1].indexOf(":") > -1) {
                String[] insideParen = splitByParen[1].split(":");
                for (int j = 0; j < insideParen.length; j++) {
                    if ((insideParen[j].matches("[0-9]+"))) {
                                // within the parenthesis, the first number is the line number, for both javaStack and jsStack
                                lineNumber = Integer.parseInt(insideParen[j]);
                                break;
                    } else {
                                // within the parenthesis, everything before first number is the fileName
                                fileName += insideParen[j] + ":";
                        }
                }
                // remove extra ":"
                fileName = fileName.substring(0, fileName.length() - 1);
            } else {
                fileName = splitByParen[1];
                lineNumber = 0;
            }

            String declaringClass;
            String methodName;
            if (splitByParen[0].indexOf(splitByParen[1].split("\\.")[0]) > -1) {
                declaringClass = splitByParen[0].substring(0, splitByParen[0].indexOf(splitByParen[1].split("\\.")[0]) - 1).trim();
                methodName = splitByParen[0].substring(splitByParen[0].indexOf(splitByParen[1].split("\\.")[0]));
            } else {
                declaringClass = splitByParen[0].substring(0, splitByParen[0].lastIndexOf('.')).trim();
                methodName = splitByParen[0].substring(splitByParen[0].lastIndexOf('.') + 1);
            }

            trace[i+1] = new StackTraceElement(declaringClass, methodName, fileName, lineNumber);
        }

        return trace;
    }

    @Kroll.onAppCreate
    public static void onAppCreate(TiApplication app) {
        KrollRuntime.setPrimaryExceptionHandler(new TiExceptionHandler(){
            @Override
            public void handleException(ExceptionMessage error)
            {
                Throwable th = new Throwable();
                th.setStackTrace(generateStackTrace(error.javaStack, error.jsStack, error.sourceName, error.message, error.lineSource, error.line));
                FirebaseCrashlytics.getInstance().recordException(th);
                super.handleException(error);
            }
        });
    }
m1ga commented 3 years ago

@petenathan42 would you mind cleaning that up (tabs/spaces), remove the jsHeader/javaHeader to make it a bit cleaner, remove the log.i and then submit a PR?

petenathan42 commented 3 years ago

@m1ga https://github.com/hansemannn/titanium-crashlytics/pull/39

petenathan42 commented 3 years ago

@m1ga Also hoping to get this one into firebase.performance https://github.com/hansemannn/titanium-firebase-performance/pull/4

m1ga commented 3 years ago

Merged & fixed. Thanks @petenathan42 :+1: