[firebase_remote_config] Remote config/double instance error

[IchordeDionysos]

Description

This pull request addresses an issue with a test where two instances were created shortly after another leading to a Future already completed StateError.
This was caused due to side-effects of the tests where all tests passed when they were executed at once and the doubleInstance test failed when executed alone.

Also in this Pull Request is a fix for the doubleInstance error. Realized with a try/catch for only this exact error. I've used this fix because to me it feels much more elegant compared to a fix where I would basically would have to check for the same shortly after.

Alternative solution:

if (!instanceCompleter.isCompleted) {
  final _remoteConfigInstance = await _getRemoteConfigInstance();
  if (!instanceCompleter.isCompleted) {
    instanceCompleter.complete(_remoteConfigInstance);
  }
}

Related Issues

#195
flutter/flutter#38060
#28

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• If the pull request affects only one plugin, the PR title starts with the name of the plugin in brackets (e.g. [cloud_firestore])
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (flutter analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
• No, this is not a breaking change.

[IchordeDionysos]

Is the failing check known to behave flakily? 🤔
I can't figure out what code changes the check failure would have triggered.
Also, it says fatal: repository 'https://cdn.cocoapods.org/' not found...

[IchordeDionysos]

Also, I'm open to alternative solutions to fix the error "Future already completed",

I just wanted to fix the tests
and get the bug fix going,
because when I attach a debugger it always stops first at the exception/error and not my breakpoint, which is annoying.

Interestingly, this does work and gives back a RemoteConfig instance it's just that the error still occurs.

[collinjackson]

We're generally trying to avoid @VisibleForTesting APIs since they clutter the public logs, though I don't see another solution at this point.

[IchordeDionysos]

Totally agree, I tried to find an alternative first before using it, but have to change it back somehow...

[IchordeDionysos]

@FirebaseExtended/invertase @collinjackson is there any other solution to be able to test for this edge case?

[IchordeDionysos]

@collinjackson @kroikie Any updates on this?
Would it be okay to use @VisibleForTesting in this case?

[IchordeDionysos]

@FirebaseExtended/invertase @Ehesp

It would be cool to have this fix landed. I've heard a couple of times in our company alone, that people spent quite some time getting to fix this issue, just because it sends a false-positive.

This adds a missing test case, where Remote Config is initialized twice in parallel, which leads to an error.

The current status of this PR is that @collinjackson wasn't too happy about using @visibleForTesting, but we found no other way to do so.
To fix the now failing test, I made some adjustments that there's no Future already completed error anymore.

I'll be happy to update the CHANGELOG.md if you are happy with the current implementation of the tests.

[IchordeDionysos]

Closing in favor of #4186 which makes this PR obsolete!