Flutterfire: 🐛 [Auth] authStateChanges() stream fires once too often upon attaching

[puf]

Bug report

Upon attaching a listener to authStateChanges() it should fire immediately with the current state, and then after that whenever the state changes. When I attach a listener, it fires twice with the same state.

Steps to reproduce

Attach a listener to the authStateChanges() stream:

FirebaseAuth.instance.authStateChanges().listen((user) {
  print('Auth state changed to '+(user!=null?user.uid:'<null>'));
}

When you run this (and a user is signed in), you'll notice it fires/prints twice.

---

Now open the underlying iOS app in Xcode and add this code in AppDelegat.didFinishLaunchingWithOptions:

Auth.auth().addStateDidChangeListener { (auth, user) in
    print("Swift addStateDidChangeListener")
    print(user != nil ? user!.uid : "<null>")
}

When you now run the app again, you'll notice that the Swift code only logs once, but the Flutter code logs twice.

Additional context

I think the problem is most likely caused by this streamController.add(currentUser); in: https://github.com/FirebaseExtended/flutterfire/blob/master/packages/firebase_auth/firebase_auth/lib/src/firebase_auth.dart#L224-L242, as the underlying Platform SDK already does precisely that when a listener is attached. So since FlutterFire passes the listener on to the platform SDK, it doesn't have to repeat firing the initial state.

---

Flutter doctor

Run flutter doctor and paste the output below:

Click To Expand

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel beta, 1.22.0, on Mac OS X 10.15.5 19F101, locale en)

[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.3)
[✓] Xcode - develop for iOS and macOS (Xcode 11.5)
[✓] Chrome - develop for the web
[✓] Android Studio (version 3.6)
[✓] VS Code (version 1.50.1)
[✓] Connected device (3 available)

• No issues found!

---

Flutter dependencies

Run flutter pub deps -- --style=compact and paste the output below:

Click To Expand

Dart SDK 2.10.0
Flutter SDK 1.22.0
hands 1.0.0+1

dependencies:
- cupertino_icons 1.0.0
- firebase_auth 0.18.1+2 [meta firebase_core firebase_core_platform_interface firebase_auth_platform_interface firebase_auth_web flutter]
- firebase_database 4.1.1 [flutter firebase_core]
- flutter 0.0.0 [characters collection meta typed_data vector_math sky_engine]
- google_sign_in 4.5.5 [google_sign_in_platform_interface flutter meta google_sign_in_web]

dev dependencies:
- flutter_test 0.0.0 [flutter test_api path fake_async clock stack_trace vector_math async boolean_selector characters charcode collection matcher meta source_span stream_channel string_scanner term_glyph typed_data]

transitive dependencies:
- async 2.5.0-nullsafety.1 [collection]
- boolean_selector 2.1.0-nullsafety.1 [source_span string_scanner]
- characters 1.1.0-nullsafety.3
- charcode 1.2.0-nullsafety.1
- clock 1.1.0-nullsafety.1
- collection 1.15.0-nullsafety.3
- fake_async 1.2.0-nullsafety.1 [clock collection]
- firebase 7.3.2 [http http_parser js]
- firebase_auth_platform_interface 2.1.1 [flutter meta firebase_core plugin_platform_interface]
- firebase_auth_web 0.3.1+1 [flutter flutter_web_plugins firebase meta http_parser intl firebase_core firebase_auth_platform_interface js]
- firebase_core 0.5.0+1 [firebase_core_platform_interface flutter quiver meta firebase_core_web]
- firebase_core_platform_interface 2.0.0 [flutter meta plugin_platform_interface quiver]
- firebase_core_web 0.2.0 [firebase firebase_core_platform_interface flutter flutter_web_plugins meta js]
- flutter_web_plugins 0.0.0 [flutter characters collection meta typed_data vector_math]
- google_sign_in_platform_interface 1.1.2 [flutter meta quiver]
- google_sign_in_web 0.9.2 [google_sign_in_platform_interface flutter flutter_web_plugins meta js]
- http 0.12.2 [http_parser path pedantic]
- http_parser 3.1.4 [charcode collection source_span string_scanner typed_data]
- intl 0.16.1 [path]
- js 0.6.2
- matcher 0.12.10-nullsafety.1 [stack_trace]
- meta 1.3.0-nullsafety.3
- path 1.8.0-nullsafety.1
- pedantic 1.9.2 [meta]
- plugin_platform_interface 1.0.3 [meta]
- quiver 2.1.3 [matcher meta]
- sky_engine 0.0.99
- source_span 1.8.0-nullsafety.2 [charcode collection path term_glyph]
- stack_trace 1.10.0-nullsafety.1 [path]
- stream_channel 2.1.0-nullsafety.1 [async]
- string_scanner 1.1.0-nullsafety.1 [charcode source_span]
- term_glyph 1.2.0-nullsafety.1
- test_api 0.2.19-nullsafety.2 [async boolean_selector collection meta path source_span stack_trace stream_channel string_scanner term_glyph matcher]
- typed_data 1.3.0-nullsafety.3 [collection]
- vector_math 2.1.0-nullsafety.3

---

[Ehesp]

Hey @puf

From memory, the reason why we added the streamController.add(currentUser); line was because the native listener is always listening, whether or not the user has. When the user subscribes, we essentially forwarded on the native events to any attached listener. The first time around it seems the client fires, and then native. After that, I believe it would only be the client (to mimic expected functionality).

I'll see what we can do to improve this.

[puf]

Thanks for clarifying @Ehesp . I assumed you must have had a good reason to add this code, as the comment above indicated a clear intent. Why is the native listener always listening? Wouldn't it be easier to attach a native listener for each listener that is attached to the stream/each time the stream is requested?

[Salakar]

Wouldn't it be easier to attach a native listener for each listener that is attached to the stream/each time the stream is requested?

Gets a little harder I think as we'd would need to track these 'pairings' of Stream to native listeners in Dart & native with some kind of identifier so native events are forwarded to correct stream instances (as these would channel through a single native <-> dart vm event channel anyway). The cleaning up/tracking of this also gets a little messy with multiple Firebase apps and things like hot reload/restart in Dart (which native then needs to be made aware of when it happens to remove listeners).

Still doable but a lot more involved I think versus a singleton auth listener per Firebase app. Unless I'm over complicating it 😅

I wonder if we could just internally on Dart discard (still handle it for the internals management of user state in Dart, but just not forward it on to the users stream) the first native event that comes through the method channel? But i'm also thinking we may have issues with hot reload/restart there as well so maybe that needs to happen on native instead

[puf]

Thanks for explaining Mike. While dropping the initial event will solve the case I flagged here, I'm still afraid that we're replicating business logic in the library.
How about having a FlutterFire wrapper for each listener, and then leaving it to the application code to manage that listener/wrapper (same as the native SDKs do)? So that way there's a 1:1 relationship between a flutter listener and its underlying native listener. Would the library then still need to track all pairings?

[Salakar]

Would the library then still need to track all pairings?

I think we still may have to for hot reloading/hot restarting or maybe do a blanket remove all - not 100% sure, would need to spend some time looking into it

I'm gonna ship the initial fix to stop the events sending twice and then I'm going to look into this further in the next few weeks and see if I can get it working ok with a 1:1 relationship as suggested

[puf]

Thanks Mike!

On Tue, Nov 24, 2020 at 12:18 PM Mike Diarmid [e-mail]
wrote:

Reopened #4049
https://github.com/FirebaseExtended/flutterfire/issues/4049.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/FirebaseExtended/flutterfire/issues/4049#event-4034649769,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAG7BXYJ3UTAGK6YMRAH5ODSRQIILANCNFSM4TMAUBPQ
.

[cbenhagen]

https://github.com/FirebaseExtended/flutterfire/pull/4099 fixed this on Android, macOS and iOS. Web still fires once too often.