bug: publish messages without being subscribed

[richard-ramos]

As described in status-im/status-go#5759 (comment) by @igor-sirotin
The code as it is right now would allow publishing a message while not being subscribed to a pubsub topic.

I suggest either checking if a subscription to the pubsub topic before publishing and return an error or maybe attempt to subscribe with this code. I kinda prefer to return an error instead since then the user of the library can choose to subscribe or not depending on the error (and also because publishing without being subscribed does look like an incorrect usage of gowaku)

//Check if gossipsub subscription already exists for pubSubTopic
		if !w.IsSubscribed(pubSubTopic) {
			_, err := w.subscribeToPubsubTopic(cFilter.PubsubTopic)
			if err != nil {
				//TODO: Handle partial errors.
				w.log.Error("failed to subscribe to pubsubTopic", zap.Error(err), zap.String("pubsubTopic", cFilter.PubsubTopic))
				return nil, err
			}
		}

[kaichaosun]

I found this piece of code which check similar logic, wondering why is it not working in this case?
https://github.com/waku-org/go-waku/blob/master/waku/v2/protocol/relay/waku_relay.go#L285

[igor-sirotin]

wondering why is it not working in this case?

I think it works 🙂 Check the logs I attached in status-im/status-go#5759 (comment) - could not send message: not enough peers to publish.

The question is, should we maybe auto-subscribe to topic in such case.

[kaichaosun]

should we maybe auto-subscribe to topic in such case.

It seems we need a wrapper API for auto subscribe. what do you think? @richard-ramos

[chaitanyaprem]

should we maybe auto-subscribe to topic in such case.

It seems we need a wrapper API for auto subscribe. what do you think? @richard-ramos

it may not be so straight-forward, because when you subscribe to a pubsubtopic doesn't mean you have peers connected wrt topic. So, doing it as part of publish may not solve the issue rather mislead further.

either we subscribe and wait to have atleast 1 peer for topic(which itself is not gauranteed msg will be reliably propagated).
or return an error so that caller can subscribe to topic and wait for it to be healthy before publishing.
wondering if we ca use topic-health to decide whether to publish or not rather than min-peers. wdyt @richard-ramos

[richard-ramos]

I agree with Prem's observations about creating a wrapper not being straightforward. Subscribing to the principle of least surprise, i'd rather have the publish function return an error and handle it appropriately.

wondering if we ca use topic-health to decide whether to publish or not rather than min-peers

IMO yes. Using topic health is a pending topic we have for go-waku / status-go integration, so perhaps this is a good moment to work on this task!