Bonsoir icon indicating copy to clipboard operation
Bonsoir copied to clipboard

A Bonsoir service has been lost after 3500ms of discovering

Open DJMontanova opened this issue 11 months ago • 9 comments

Describe the bug When I'm trying to discover _webrtc._udp. protocols on my local network, the discovery finds all the devices, but loses them after 3500ms, and re-finds them right after. The problem is that I resolve them when they appear for the first time to get their IP. When the service is lost and re-found, it is already resolved, but its attributes are scrambled with attributes from another service.

image On this image, you can see that the service is found et resolved, with clearly different attributes. After the service is lost and re-found at the bottom of the screen, the attributes are mixed.

To Reproduce Steps to reproduce the behavior: It can be reproduced by cloning the example project : https://github.com/Skyost/Bonsoir/tree/master/packages/bonsoir Broadcast 2 services with the same type and different attributes, and launch a discovery for that type. Better debug logs while did on an android smartphone due to the debug console.

Expected behavior I do not want to lose the services after 3500ms.

Smartphone :

  • Device: Oppo Find X3 Pro
  • OS: Android 14, API 33

DJMontanova avatar Jan 08 '25 15:01 DJMontanova

Hi,

this seems to be related to the usage/implementation of queryTxtRecord(). On each serviceFound event there is a call to queryTxtRecords without needing to resolve the service. When it resolves the TXT records, it produces one serviceLost event and then one serviceFound event after changing the service attributes to the result of the query attempt.

I am not sure what is the intention behind this originally however it's seems to be doing more harm then good as I would like to see only real mdns events (eg. serviceFound and serviceLost are the same as for example when you run avahi-browse).

This queryTxtRecord should either produce different event (something like txtRecordsResolved) or the API should provide a way to disable this query.

Do you have any thoughts about this @Skyost ?

Daniel

foxik0169 avatar Feb 10 '25 11:02 foxik0169

On Android (only), it's impossible to query service properties without resolving them first (see NsdServiceInfo#getAttributes()). Because of this limitation, we need to query TXT records separately when a service is discovered.

How should we handle this? There are two possible approaches:

  1. Query TXT records before reporting the service. We could delay reporting the service until TXT records have been retrieved. However, this introduces a delay (up to 2 seconds, due to timeout), meaning the service would only be reported after the query succeeds or fails.

  2. Query TXT records asynchronously after discovering the service. Instead of delaying, we perform the TXT record lookup in the background. If the query succeeds, we trigger a service removed / service found sequence to update the service information. This is the approach I've chosen.

Additionally, we can cancel the TXT record resolution if a resolve() call is made in the meantime.

Skyost avatar Feb 10 '25 12:02 Skyost

Yeah, canceling the TXT resolution when resolve() is called sounds reasonable.

foxik0169 avatar Feb 10 '25 12:02 foxik0169

That being said, producing serviceLost and serviceFound events still feels not correct - it doesn't represent the reality of the network. Maybe letting the user specify if he wants to run discovery with or without resolving TXT records on start and then when he chooses to resolve TXT records, produce serviceFound event after the TXT record has been resolved.

// edit

Another alternative may be to produce "serviceChanged" events as that seems more appropriate even for real service change events from native implementations.

foxik0169 avatar Feb 10 '25 13:02 foxik0169

That being said, producing serviceLost and serviceFound events still feels not correct - it doesn't represent the reality of the network. Maybe letting the user specify if he wants to run discovery with or without resolving TXT records on start and then when he chooses to resolve TXT records, produce serviceFound event after the TXT record has been resolved.

That adds some unneeded complexity, in my opinion.

Another alternative may be to produce "serviceChanged" events as that seems more appropriate even for real service change events from native implementations.

The problem is that this event would only fire on Android...

Skyost avatar Feb 14 '25 12:02 Skyost

That adds some unneeded complexity, in my opinion.

That's true. I am trying to find a way so that the user of the API would know, that the device actually was not lost in theese specific events. Basically to get to what native API provide - MDNS events as they appear on network.

The problem is that this event would only fire on Android...

Changes to TXT records on services on iOS are also implemented by dispatching a pair of serviceLost and serviceFound events if I understand it correctly?

I guess, on iOS the first time the device is discovered it's after bonjour finds first TXT records and then all changes are reported, because it supports this natively.

foxik0169 avatar Feb 14 '25 13:02 foxik0169

Changes to TXT records on services on iOS are also implemented by dispatching a pair of serviceLost and serviceFound events if I understand it correctly?

On iOS, services are directly reported with their TXT record. On Android, we have to resolve them manually.

But, you'va made a point : iOS and macOS support reporting changes... so that a serviceChanged event would not be useless on Darwin platforms. Maybe we could add a serviceChanged event, as you've suggested, and document what it does on which platform.

Skyost avatar Feb 14 '25 13:02 Skyost

I have encountered the same issue. In this case, it is not easy to judge logically whether the device is maintaining the connection. Has there been any progress on this issue?

zhponng avatar May 29 '25 01:05 zhponng

There has been no progress on this issue, because I've been quite busy by my work recently. I will have the time to do this (and to do various other needed updates as well) during either june or july. Feel free to submit your PR tho.

Skyost avatar May 30 '25 12:05 Skyost