-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.activity
can be null with dynamic links since it is possible to open the app and execute this code before the activity is ready.