Weird 'subscribe failed' exceptions

[willbryant]

We're using the 4.0 beta. We're seeing some weird generic _Exception exception error reports with the message subscribe failed and no backtrace.

~They get wrapped in a RequestOtherException which does have a backtrace, but it's just RequestHandler.response (package:pubnub/src/networking/request_handler/io.dart:139).~ Turns this bit was inapplicable, that was Sentry grouping too many errors together. We have no backtrace that I can see.

I can't find this error actually get thrown anywhere (not in the pubnub library nor our app). But, there is this inside SubscribeLoop's _loop():

        if (diagnostic == null) {
          _logger.warning('No diagnostics found.');

          if (!_whenStarts.isCompleted) {
            _whenStarts.completeError(Exception('subscribe failed'));
          }
          rethrow;
        }

(Coincidentally, this is the same method involved in #63 which @are is looking into)

Since that Exception passed to completeError isn't actually thrown, I guess that would explain the lack of a backtrace?

But even if this is the source of the exception, I still don't understand:

• Under what conditions the subscribe loop goes into this !isCompleted case when an exception is caught - actually, it looks to me like it will run this completeError every time, because _whenStarts is reset to a new Completer as soon as it is completed in the code above
• How this error completion gets thrown later. We don't have any references to whenStarts or _whenStarts, and although the library itself has some delegations to expose it, I don't see anything in the library actually using it.

It's almost as if the exception that is created but not raised becomes the exception that rethrows throws? But that feels hard to believe!

[willbryant]

OK, I think I've figured this out, I should have realised when I said "We don't have any references to whenStarts or _whenStarts, and although the library itself has some delegations to expose it, I don't see anything in the library actually using it".

That's exactly the problem - nothing is awaiting the future (and catching errors). Whenever a future that has no "successor" completes with an error, the Dart "unhandled exception" error handler gets called, which in our case logs this error.

Nothing to do with rethrow at all, my mistake.

I do think the reason there's no backtrace is that the exception never got raised. That plus the fact that it's a completely generic exception class and message makes it pretty hard for a new dev to see that this error comes from the Pubnub library, and that it's actually not a significant problem (since it isn't a rethrow, it's not breaking the loop like #63).

Can I suggest that:

1. A more specific exception type (and maybe message) be used so it's easier to understand the error and easier to ignore in error reporting tools (could maybe even just completeError with the existing exception object?)
2. You consider adding a dummy onError/catchError on the future; not on the one that gets returned from whenStarts, where I guess the idea was for callers to be able to see that an error had occurred, but just one to absorb the error so it doesn't go to the "uncaught exception" handler.

I reckon you could probably just do something like this: Future _dummy = _whenStarts.future.catchError((_error) => null); (but obviously you need to make sure that gets actually run after each recreation, not just put it in a getter like the Future<void> get whenStarts => _whenStarts.future;, so not sure how best to fit this in to the current class).

[are]

Thanks again for reporting this issue and doing such an extensive research into this!

I'm thinking on how to handle this and maybe a Future is not a good interface for whenStarts after all, what do you think about a Stream? It could be stored internally with a proper error handler as to avoid unhandled exceptions and the whenStarts getter could be just Future<void> get whenStarts => _whenStartsController.stream.take(1).first.

[willbryant]

I'm not really very clear on the intended use case, is it to get rid of "Connecting..." status messages?

A stream sounds reasonable to me. I think you can use _whenStartsController.stream.first without the take?

Not keen to expose the stream itself?

[are]

whenStarts was created because some customers wanted to know when the su.bscription actually starts "listening" for messages so they can synchronize some of the things they wanted to do. Also because Dart SDK doesn't have status messages, this is supposed to alleviate some of that.

We can expose the stream itself, but when we do then the user needs to manage a subscription. With the getter exposed only you don't need to worry about the stream itself.

I think the take(1) is important because you want the first event you receive AFTER you "call". The stream won't be recreated the way the future currently is, so if you use whenStarts after a 3rd loop then .stream.first will no longer be useful (I might be wrong, I'll test this to be sure).

[willbryant]

Any progress on this ticket? Still seeing it happen.

[github-actions[bot]]

@willbryant this issue is addressed in v4.0.0