Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App Distribution FIRAppDistributionRelease.releaseNotes may be null, documented as non-null #8602

Closed
mikehardy opened this issue Aug 31, 2021 · 5 comments

Comments

@mikehardy
Copy link
Contributor

Step 0: Are you in the right place?

  • For issues or feature requests related to the code in this repository
    file a Github issue.
    • If this is a feature request please use the Feature Request template.
  • For general technical questions, post a question on StackOverflow
    with the firebase tag.
  • For general (non-iOS) Firebase discussion, use the firebase-talk
    google group.
  • For backend issues, console issues, and other non-SDK help that does not fall under one
    of the above categories, reach out to
    Firebase Support.
  • Once you've read this section and determined that your issue is appropriate for
    this repository, please delete this section.

[REQUIRED] Step 1: Describe your environment

  • Xcode version: 12.5.1
  • Firebase SDK version:8.6.0
  • Installation method: CocoaPods
  • Firebase Component: App Distribution (Auth, Core, Database, Firestore, Messaging, Storage, etc)

[REQUIRED] Step 2: Describe the problem

Steps to reproduce:

The documentation for the releaseNotes property on FIRAppDistributionRelease indicates it is non-null https://firebase.google.com/docs/reference/ios/firebaseappdistribution/api/reference/Classes/FIRAppDistributionRelease#releasenotes

However, in practice the property may be null? invertase/react-native-firebase#5667

Now, it may well be this is my incorrect interpretation of what non-null means in objective-c as a modifier on a property, perhaps what it means is that you cannot attempt to set it to null, but if accessed (without ever being set) it is potentially null.

If so, apologies for the noise, please close, and I've learned something (and thank you)

But if this is an issue it should be fixed either in documentation or code or where ever the data is initiall provided I guess

Relevant Code:

Crash report in linked react-native-firebase issue, but:

resolve(@{
      @"displayVersion" : release.displayVersion,
      @"buildVersion" : release.buildVersion,
      @"releaseNotes" : release.releaseNotes,
      @"isExpired" : [NSNumber numberWithBool:release.isExpired],
      @"downloadURL" : release.downloadURL
    });

If you do that, the releaseNotes property will potentially be nil

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@mikehardy
Copy link
Contributor Author

For ecosystem comparison, I know firebase-android-sdk hasn't released app distribution yet, but they are working hard on it, and they are marked as nullable:

https://github.com/firebase/firebase-android-sdk/blob/82b02af331d4674682bfb90c195da45543b808c3/firebase-app-distribution/src/main/java/com/google/firebase/appdistribution/AppDistributionRelease.java#L46-L48

mikehardy added a commit to invertase/react-native-firebase that referenced this issue Aug 31, 2021
mikehardy added a commit to invertase/react-native-firebase that referenced this issue Aug 31, 2021
@paulb777
Copy link
Member

paulb777 commented Oct 1, 2021

This may be addressable earlier, but I'll mark for the next major release to review when we do a thorough nullability review at that point.

@paulb777 paulb777 added this to the Firebase 9 milestone Oct 1, 2021
@ncooke3 ncooke3 self-assigned this Mar 9, 2022
@ryanwilson ryanwilson assigned ryanwilson and unassigned ncooke3 Mar 31, 2022
@ryanwilson
Copy link
Member

Confirming with the App Distro team that this is intentional and not a backend bug or anything - will fix afterwards.

ryanwilson added a commit that referenced this issue Mar 31, 2022
These were unintentionally marked as non-null, but should be nullable.

Fixes #8602
ryanwilson added a commit that referenced this issue Mar 31, 2022
These were unintentionally marked as non-null, but should be nullable.

Fixes #8602
@ryanwilson
Copy link
Member

Fixed in the Firebase 9 branch which will be merged soon. Closing now, thanks for the report and sorry for the troubles!

paulb777 pushed a commit that referenced this issue Apr 4, 2022
These were unintentionally marked as non-null, but should be nullable.

Fixes #8602
@firebase firebase locked and limited conversation to collaborators May 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants