[firebase_dynamic_links] Don't crash if activity is missing #1989
Conversation
|
@bparrishMines I am not sure if that call is needed for registering OnSuccessListener. There are some public examples where only the listener is passed in. Is there a way to test this? |
| @@ -117,6 +115,11 @@ public void onMethodCall(MethodCall call, Result result) { | |||
| } | |||
|
|
|||
| private void handleGetInitialDynamicLink(final Result result) { | |||
| if (registrar.activity() == null) { | |||
| result.success(null); | |||
There was a problem hiding this comment.
I think we should use result.error() here. Wouldn't the user want to know that it failed because there was no activity?
There was a problem hiding this comment.
If the activity has not started, then there is no initial link. I think it is WAI that it returns null.
|
@mehmetf We haven't written tests for this yet. I've been testing it manually. I can do that for you if you need me to. Although, I believe the listeners don't require an |
|
Thanks @bparrishMines . I would really appreciate it if you gave it a whirl. I am not sure how to test it out. |
|
Oh! I just remembered that we are in the middle of moving flutterfire plugins to a new repo https://github.com/firebaseextended/flutterfire. @collinjackson All changes should be added to firebaseextended now, right? Or do we plan on cherry-picking until #1984 lands. |
|
Also, i tested your PR and the plugin still works as intended. |
|
Thanks Maurice. I am OK submitting this to the other repo as well if you want to close this. I made this change internally already but the next time we sync plugins, we need to point to the other repo. Let me know when that work is complete, @collinjackson . |
|
I am going to close this and redo it in the new repo. |
registrar.activitycan be null with dynamic links since it is possible to open the app and execute this code before the activity is ready.