Adding assertions and documentation for 15 char limiton ty_domain

[FelixSelter]

Hi,
I am new to this and struggled for the past 3 hours trying to find a simple mistake I made.

    const MDNS_SERVICE_NAME: &'static str = "_VERYLONGNAME._tcp.local.";

    let service_info = ServiceInfo::new(
        MDNS_SERVICE_NAME,
        &instance_name,
        &host_name,
        local_ip().map_err(|e| BackendError::new("Could not get local ip", &format!("{}", e)))?,
        MDNS_PORT,
        &MDNS_PROPERTIES[..],
    )

I used a long name inside MDNS_SERVICE_NAME. If this VERYLONGNAME part above is longer than 15 characters nothing will work you wont get any errors and it just silently fails. This is not mentioned in the code of ServiceInfo and not in docs.rs in the example.

I think we should add a comment to the ty_domain property on ServiceInfo, add assertions to verify this inside ServiceInfo::new, also add this in the docstring of new and add information for the module documentation for the example shown on docs.rs

If you confirm I could do that

[keepsimple1]

I'm sorry that it wasted your time. I hear you. The background is that the current check is in the daemon thread and return the error via a monitor channel in register_service.

It definitely makes sense to improve this.

I think we should add a comment to the ty_domain property on ServiceInfo,

agreed!

add assertions to verify this inside ServiceInfo::new,

Because we support API of set_service_name_len_max in the daemon thread, this is not easy to check it directly in ServiceInfo::new (the reason we didn't have it). Currently we only support checking such errors via monitor channel. Any ideas ?

also add this in the docstring of new and add information for the module documentation for the example shown on docs.rs

agreed!

If you confirm I could do that

That'll be great! Thanks in advance!

[FelixSelter]

Hi thank you for your fast reply.

I quickly setup logging and this does indeed work.
[2026-02-01T08:40:25Z DEBUG mdns_sd::service_daemon] check_service_name_length: Service name length must be <= 15 bytes

yesterday I also enabled logging but I should have disabled traces they cluttered everything and I never found this. Probably because this is a debug and no warning or error.

I did a quick glance over the code (no experience with mDNS though).
As far as I understand it:

1. We spin up a daemon thread that does all of the heavy lifting
2. register only sends a command to that thread with no direct response
3. The thread will execute this command whenever it feels ready and has no way to communicate back the error directly to the caller
4. It sends it via this monitor channel so at least someone is notified (if anyone subscribed)
5. We cannot hardcode an assertion because the length could have be changed by set_service_name_len_max which is only known to Zeroconf which only the thread knows about

If that understanding is correct, setting up such a monitor connection seems like generally a good idea or does it come with a large performance penalty? Maybe I could update the examples to show how to setup such a connection. Then anyone using the library could see at first glance that this is possible and recommended.

Is there any reason why the log is a debug and nothing more significant? Can this be changed? It silently breaks everything.

The current design makes it indeed hard to catch this. If we wanted to make it more obvious than a doc string we could add something like this with better wording:

#[deprecated(
    note = "Warning: mDNS service name label cannot exceed 15 bytes. \
            This may silently fail unless you changed the daemon limit. \
            You may silence this warning with #[allow(deprecated)]." 
)]

I think other libs use that pattern as well but I dont know if you want to go that route and adding the comments and examples would already be a big improvement. I currently see no other options.

I think I can work on this on tuesday evening german time UTC+1.

Best regards

[FelixSelter]

@keepsimple1 can you please confirm?

[FelixSelter]

#431 I did most of the changes discussed above excluding the deprecation message

[keepsimple1]

Sorry I missed this reply for some reason.

If that understanding is correct, setting up such a monitor connection seems like generally a good idea or does it come with a large performance penalty?

Not real performance penalty.

Maybe I could update the examples to show how to setup such a connection. Then anyone using the library could see at first glance that this is possible and recommended.

Sounds good!

Is there any reason why the log is a debug and nothing more significant? Can this be changed? It silently breaks everything.

The reason is that most Rust programs set INFO level logging by default. This library also has logging feature enabled by default. So if we use any INFO or above logging, it will very likely show up in the user's logs by default. That, in my experience, seems not welcomed by most users.

At this moment, I'd prefer not to change this. We can document it a bit better if that helps.

#[deprecated(
note = "Warning: mDNS service name label cannot exceed 15 bytes.
This may silently fail unless you changed the daemon limit.
You may silence this warning with #[allow(deprecated)]."
)]

Sorry I don't understand this part. Where will we add this attribute?

adding the comments and examples would already be a big improvement.

Yes!

#431 I did most of the changes discussed above excluding the deprecation message

Thank you! Let me take a look at that.

EDIT: I think the error level log you added is fine.

[FelixSelter]

No worries. I will update the pr according to your comments.

The depreciation attribute could be added to the ServiceInfo::new function. If anyone would use that function their id would mark it as deprecated (strike through or similar) Naturally anyone will hover over it and review the message informing them over the character limit. They could then mark it as ignore deprecated.

This is just a pattern I saw somewhere else but it might be a bit excessive. I was just adding this as an option because you were asking for other ideas.

[keepsimple1]

The depreciation attribute could be added to the ServiceInfo::new function.

Like you also said, I think we don't need that in this case, because ServiceInfo::new is not actually deprecated.

[FelixSelter]

Okay great. One other option would be to actually dual store the max length. We could add a property max length to ServiceDaemon, update it whenever someone calls set max length and inside register we could validate the ServiceInfo and throw a runtime error. If we wanted to avoid the tiny overhead we could add flags so it's only added inside debug builds with #[cfg(debug_assertions)] I think. Would that be an option or do you think we did enough?

Thank you for providing the important lines by the way. That was really helpful.

[keepsimple1]

Would that be an option or do you think we did enough?

I think we did enough. (We / I will know better after we release these basic improvements..)