🐛 Bug Report: Realtime.subscribe method doesn't dispatch any error if try to subscribe without connection.

[FernandoUFS]

👟 Reproduction steps

When using realtime.subscribe without connection it's not possible detect if it succeeded or failed, because the error of connection is not dispatched to outside of the sdk.

The try catch is never fired nor the stream.onError nor the stream.onDone when trying to subscribe. Example:

try {
  RealtimeSubscription _messagesRealtimeSubscription = realtime.subscribe(['databases.chat.collections.${widget.group}.documents']);

  _chatSubscription = _messagesRealtimeSubscription.stream.listen((event) {
    Log.w(event.toString(), prefix: 'chat');
  }, onError: (e, st) {
    Log.e(e.toString(), st: st, prefix: 'chat');
  }, onDone: () {
    Log.e('Connection closed', prefix: 'chat');
  });
} catch (e, st) {
  Log.e(e, st: st, prefix: 'chat');
}

👍 Expected behavior

It should be possible to catch connection error and it should be possible to be sure that the connection was stablished.

When I say "to be sure that the connection was stablished" I mean that knowing if the stream had an error only solves one part of the problem. This is because I cannot be sure that the connection was successful simply by expecting not to receive an error.

Maybe the subscribe method being a Future I can know when the connection was stablished by awaiting. For instance:

try {
    await realtime.subscribe(['databases.chat.collections.${widget.group}.documents']);
    log('success');
    showSuccessToUser();
} catch (e, st){
    log(e.toString);
    showErrorToUser();
    startAutoReconnectApproach();
}

👎 Actual Behavior

It actually never returns any errors in any way. The onDone function is only called if the connection was successfully established before and then the connection dies. However, if you try to subscribe without a connection, no error is dispatched.

🎲 Appwrite version

Version 2.0.x

💻 Operating system

MacOS

🧱 Your Environment

flutter doctor

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.7.6, on macOS 13.1 22C65 darwin-arm64, locale en-BR)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2022.1)
[✓] VS Code (version 1.76.0)
[✓] Connected device (2 available)
[✓] HTTP Host Availability

pubspec.yml

dependencies:
  appwrite: ^8.3.0

Device

Galaxy S22 with Android 13

👀 Have you spent some time to check if this issue has been raised before?

• [X] I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

• [X] I have read the Code of Conduct

[stnguyen90]

@FernandoUFS thanks for creating this issue! 🙏🏼 I think this problem might be here:

https://github.com/appwrite/sdk-for-flutter/blob/f8480fdeecfb23782bd5b33d0e2a7db44fc8c391/lib/src/realtime_mixin.dart#L117

If _createSocket() throws an exception, it won't be handled.

@lohanidamodar, what are your thoughts on how to proceed with this?

[lohanidamodar]

@FernandoUFS thanks for creating this issue! 🙏🏼 I think this problem might be here:

https://github.com/appwrite/sdk-for-flutter/blob/f8480fdeecfb23782bd5b33d0e2a7db44fc8c391/lib/src/realtime_mixin.dart#L117

If _createSocket() throws an exception, it won't be handled.

@lohanidamodar, what are your thoughts on how to proceed with this?

may be an await the Future can solve the problem?

[FernandoUFS]

@lohanidamodar I could solve this issue by two ways.

First way: The await worked, but to work I had to change the method subscribeTo from realtime_mixin.dart to return a Future<RealtimeSubscription> then, the same thing for Realtime.subscribe and its implementations. This causes a breaking change because the changing of the return type, everyone would have to do await realtime.subscribe now. I committed the changes in a fork: https://github.com/FernandoUFS/appwrite-sdk-for-flutter/commit/a0825c8d87d437648dead62e4121a5fefe94407f

Second way: This line is the one causing the error when trying connecting without connection: https://github.com/appwrite/sdk-for-flutter/blob/f8480fdeecfb23782bd5b33d0e2a7db44fc8c391/lib/src/realtime_mixin.dart#L33 Using a try catch in that point we can send the error to the streams. But also it would be necessary send a success stream, so we can be sure that the connection was stablished.

I tested with this code, and I could get the error in the realtimeSubscription stream onError.

try {
      if (_websok == null) {
        _websok = await getWebSocket(uri);
        // Create a RealtimeMessage and send to the channels informing that the connection was success.
        _lastUrl = uri.toString();
      } else {
        if (_lastUrl == uri.toString() && _websok?.closeCode == null) {
          return;
        }
        await _closeConnection();
        _lastUrl = uri.toString();
        _websok = await getWebSocket(uri);
      }
    } catch (e) {
      for (var list in _channels.values) {
        for (var stream in list) {
          stream.sink.addError(AppwriteException('Error connecting to server'));
        }
      }
    }

[lohanidamodar]

Thank you @FernandoUFS would you like to make a PR against https://github.com/appwrite/sdk-generator ? If not, no worries, we will look into it as well.

[FernandoUFS]

Hi @lohanidamodar, as you can see, I was able to solve the problem in two ways, but I think the first way (the code in the repository) is more intuitive. However, this causes a breaking change, because now everyone will need to use await realtime.subscribe for the code to work. So, I would prefer you analyze which is the best option. Thank you.

[lohanidamodar]

Thank you @FernandoUFS yes, we would not want to break the interface as well as would want to keep it consistent with other SDKs. So giving back the error on onError for streams is the better option

[moshOntong-IT]

Thank you @FernandoUFS yes, we would not want to break the interface as well as would want to keep it consistent with other SDKs. So giving back the error on onError for streams is the better option

any update on this sir?