[flutter_local_notifications] Fix native stacktraces for Android

[ueman]

This improves on #2088

As seen in the following image, the serialization of the error stacktrace doesn't work.

This can be fixed when the code is changes as described in the PlatformException.stacktrace docs.
Native exceptions on Android should not be caught, otherwise the PlatformException.stacktrace will not be populated.

This change therefore allows you to read the actual, correct stacktrace, instead of the stacktrace that's thrown during the serialization on the Android side.

Let me know if you have any questions

[MaikuB]

Thanks for the PR and raising this issue. I noticed pigeon seems to have an approach that allows for returning the error code and retain the stack trace. I'm aware that from what you've written that the error code wouldn't be returned right now but having both would help retain consistency with the rest of the plugin. For reference, this is the class that I found on this https://github.com/flutter/packages/blob/cfe0c212700bfe55690b03b094c5e2e1e399871a/packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java. Based on what I've seen in https://github.com/flutter/packages/blob/cfe0c212700bfe55690b03b094c5e2e1e399871a/packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java#L51, I suspect the simple solution is to retain most of the code that existed before and call Log.getStackTraceString(exception) as that is pigeon itself is dealing with similar scenarios and actually results in a String so should be serialisable. In other words the change would be from

result.error(UNSUPPORTED_OS_VERSION_ERROR_CODE, e.getMessage(), e.getStackTrace());

to

result.error(UNSUPPORTED_OS_VERSION_ERROR_CODE, e.getMessage(), Log.getStackTraceString(e));

I did an experiment by changing the Java code so that upon trying to show a notification that it would throw an exception due to a null default icon to then catch it and get the stack trace string like above and it worked. A screenshot of the diff in case you're curious on how to reproduce this

exception gets thrown with the expected error code ,message stack trace too

[ueman]

What you propose is definitely a good step in the right direction, but it still doesn't put the native stacktrace in the PlatformException.stacktrace property as one would expect it. I would prefer my solution, as the error code can be less accurate than the actual exception with its stacktrace.

At the end, it's your decision and either solution is better than the current situation.

[MaikuB]

Ah yes I forgot about that point you raised. In that case, from what I tried, it looks like throwing a class that extends from RuntimeException like the FlutterError class defined in pigeon seems to achieve what you're after that a PlatformException is thrown on the Dart/Flutter side, has the error code, message and the stack trace property is populated. How about that approach as it looks like it satisfies both criteria?

[ueman]

Hey, sorry for the long pause on this PR, but I was on vacation.

Ah yes I forgot about that point you raised. In that case, from what I tried, it looks like throwing a class that extends from RuntimeException like the FlutterError class defined in pigeon seems to achieve what you're after that a PlatformException is thrown on the Dart/Flutter side, has the error code, message and the stack trace property is populated. How about that approach as it looks like it satisfies both criteria?

This does not work, since this library is not using Pigeon.
While subclassing RuntimeException does also work, there's no special handling for populating the error code and the stacktrace correctly from a RuntimeException in the Flutter engine.

[MaikuB]

Hey, sorry for the long pause on this PR, but I was on vacation.

All good and no need to apologise. Personal life comes first and I'll be on vacation soon myself :)

This does not work, since this library is not using Pigeon.
While subclassing RuntimeException does also work, there's no special handling for populating the error code and the stacktrace correctly from a RuntimeException in the Flutter engine.

Have you actually tried it out though? What I mentioned was actually from me doing an experiment that would imply that the engine is handling this somewhere though I've not had a chance to dig into this further. I suspect it's from default behaviour on deserialising errors through the platform channel

[ueman]

No, I haven't tried it, but the only code path that correctly passes the stacktrace from the Android side to the Flutter side is this

https://github.com/flutter/engine/blob/e2319935d8c8fc6f1f2af21828af59a5b248eaeb/shell/platform/android/io/flutter/plugin/common/MethodChannel.java#L285-L290

and it doesn't have any special handling for a FlutterError or a FlutterException.

[MaikuB]

Yep you're right though it does make it more mysterious on how it works. Looking at code generated by pigeon and what you sent would seem to show there's an alternative that to achieve what you're after with minimal changes to the existing plugin code. By that I'm referring to calling Log.getStackTraceString(...) instead of getStackTrace on the exception. The link you shared shows that Flutter itself does this as well when it catches the RuntimeException. Would you be ok to change to this approach instead?

[ueman]

Yep you're right though it does make it more mysterious on how it works. Looking at code generated by pigeon and what you sent would seem to show there's an alternative that to achieve what you're after with minimal changes to the existing plugin code. By that I'm referring to calling Log.getStackTraceString(...) instead of getStackTrace on the exception. The link you shared shows that Flutter itself does this as well when it catches the RuntimeException. Would you be ok to change to this approach instead?

I'm not quite sure how that solves passing the stacktrace in the stacktrace property of the PlatformException. Of course, that approach works when passing the stacktrace in the details property of the PlatformException.

[MaikuB]

Yep you're right. Sorry forgot about that again 😅

[ueman]

Okay, then let's fix the serialization first, as that's still better than it being broken. Passing the stacktrace correctly can be still done later. I'll change the code in this PR later

[ueman]

io.flutter.Log isn't exposed to be used by plugins, but the implementation of Log.getStackTraceString(...) is just android.util.Log.getStackTraceString(e). So I used that.

Should I add an entry to the changelog? Is anything else missing, which I should do?

[MaikuB]

If you want to add a changelog entry then you're welcome to do so but may be a bit odd for you to do so and credit yourself but if you're comfortable with that and bump the version in the pubspec then go ahead. Otherwise I'll do it :)