diffusiondata / diffusion-examples

Diffusion API and Getting Started Examples
9 stars 13 forks source link

onClose not called in missing topic handler #9

Closed kopisa closed 8 months ago

kopisa commented 9 months ago

When closing the client session from the Diffusion console, MissingTopicNotificationStream#onClose() is never called. However, if I use MissingTopicHandler#onClose(), which is marked as deprecated, it is called.

// works
topicControl.addMissingTopicHandler(topic, new TopicControl.MissingTopicHandler() {
    @Override
    public void onMissingTopic(TopicControl.MissingTopicNotification missingTopicNotification) {
        logger.info("request: {}", missingTopicNotification.getTopicPath());
    }

    @Override
    public void onActive(String s, RegisteredHandler registeredHandler) {
        logger.info("active");
    }

    @Override
    public void onClose(String s) {
        logger.error("close"); // it's called!
    }
});

// doesn't work
topicControl.addMissingTopicHandler(topic, new TopicControl.MissingTopicNotificationStream() {
    @Override
    public void onMissingTopic(TopicControl.MissingTopicNotification missingTopicNotification) {
        logger.info("request: {}", missingTopicNotification.getTopicPath());
    }

    @Override
    public void onError(ErrorReason errorReason) {
        logger.error("error");
    }

    @Override
    public void onClose() {
        logger.error("close"); // it's not called!
    }
});

Client version: implementation 'com.pushtechnology.diffusion:diffusion-client:6.8.9' Server version: 6.8.9

(the same behavior is observed on higher versions).

dimejiogunyoye commented 9 months ago

Hi @kopisa - thanks for raising this. We'll look into this and get back to you

dimejiogunyoye commented 8 months ago

Hi @kopisa - after some discussion with our engineering team, this is behaving as expected.

TopicControl.MissingTopicHandler is a sub interface of TopicTreeHandler TopicControl.MissingTopicNotificationStream is a sub interface of Stream and Callback

With MissingTopicHandler session closures, invoked the onClose callback (as you've highlighted). Though MissingTopicNotificationStream invokes the onError callback (inherited from the Callback interface) when the session is closed. We document this here

Notification of a contextual error related to this callback. This is analogous to an exception being raised. Situations in which onError is called include the session being closed, a communication timeout, or a problem with the provided parameters. No further calls will be made to this callback for the call context.

Apologies for any confusion here. Let us know if you have any questions at all.

Thanks, Dimeji

kopisa commented 8 months ago

So if MissingTopicNotificationStream#onError is called it's the same as it being closed? No more calls on MissingTopicNotificationStream#onMissingTopic will be received? If so I suppose MissingTopicNotificationStream#onClose is never called?

dimejiogunyoye commented 8 months ago

When a missing topic notification handler is added, a Registration object is returned by the CompleteableFuture. When this registration object is closed with close() this will fire the onClose() callback for the missing topic notification stream. Documented here

Request that the handler is unregistered from the server. After the handler is unregistered, the handler's onClose method will be called. A handler can only be unregistered once. A given instance will return the same CompletableFuture if this method is called more than once.

Yes, once the onError callback has been fired for a stream, there will be no more calls made to that stream

kopisa commented 8 months ago

thanks!