Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reword does not respond messages #1354

Open
jsoref opened this issue May 31, 2024 · 6 comments · May be fixed by #1365
Open

Reword does not respond messages #1354

jsoref opened this issue May 31, 2024 · 6 comments · May be fixed by #1365
Labels
T-Question Type: Incoming question
Milestone

Comments

@jsoref
Copy link
Contributor

jsoref commented May 31, 2024

https://github.com/search?q=repo%3Azonemaster%2Fzonemaster-engine+%2Fname+%3Fserver.*+does+not+respond%2F+NOT+language%3A%22Gettext+Catalog%22+language%3APerl++NOT+path%3A%2F%5Et%5C%2F%2F&type=code

'Name server "{ns}" does not respond to an SOA query.', @_;

see #1353 for name server vs nameserver

'Nameserver {ns} does not respond to NS queries over UDP.', @_;

'Nameserver {ns} does not respond to SOA queries over UDP.', @_;

'Nameserver {ns} does not respond to any queries over UDP.', @_;

'Nameserver {ns} does not respond to NS queries over TCP.', @_;

'Nameserver {ns} does not respond to SOA queries over TCP.', @_;

'Nameserver {ns} does not respond to any queries over TCP.', @_;

'SOA MNAME name server "{ns}" does not respond to an SOA query.', @_;

Unlike most messages where zonemaster-engine has received a response and is able to critique what it receives, and can thus reasonable assert that what it got was what the server sent, when zonemaster-engine doesn't receive a response, it doesn't actually know if the server didn't receive the request, didn't send a response, or if the response was lost somewhere along the way.

It would be better to say:

Did not receive response to query for X via Y from name server {ns}

query for X via Y could be:

  • NS queries over UDP
  • NS queries over TCP
  • SOA queries over UDP
  • any queries over UDP
  • NS queries over TCP
  • any queries over TCP
  • an SOA query

name server could be:

  • name server
  • SOA MNAME name server
@matsduf
Copy link
Contributor

matsduf commented Jun 3, 2024

@jsoref, thank you for your comments. See #1353.

@matsduf matsduf added the T-Question Type: Incoming question label Jun 3, 2024
@tgreenx
Copy link
Contributor

tgreenx commented Jun 25, 2024

Unlike most messages where zonemaster-engine has received a response and is able to critique what it receives, and can thus reasonable assert that what it got was what the server sent, when zonemaster-engine doesn't receive a response, it doesn't actually know if the server didn't receive the request, didn't send a response, or if the response was lost somewhere along the way.

Right, I agree.

It would be better to say:

Did not receive response to query for X via Y from name server {ns}

Perhaps we could simply reword it as such instead:

No response received from name server {ns} to {query_type} query (or queries, if applicable)

@tgreenx tgreenx added this to the v2024.2 milestone Jun 25, 2024
@matsduf
Copy link
Contributor

matsduf commented Jun 25, 2024

Perhaps we could simply reword it as such instead:

No response received from nameserver {ns} to {query_type} query (or queries, if applicable)

Technically it is always queries, but I think query is simpler. I think that "name server" is the preferred alternative instead of "nameserver". For some test cases there should be room for "over TCP" or "over UDP".

@tgreenx
Copy link
Contributor

tgreenx commented Jun 25, 2024

Perhaps we could simply reword it as such instead:
No response received from nameserver {ns} to {query_type} query (or queries, if applicable)

I think that "name server" is the preferred alternative instead of "nameserver".

Indeed, I unintentionally misspelled. Edited.

For some test cases there should be room for "over TCP" or "over UDP".

Sure, that's fine.

@jsoref
Copy link
Contributor Author

jsoref commented Jun 25, 2024

I think I agree w/ the idea of simplifying queries to query (from an end user's perspective, and even conceptually, the client here is performing a query to the remote Domain Name System server -- it just happens to be sending a number of individual queries in order to perform that greater query action), but trying to do too many things at once seems like it's going to trip me up badly. (I've already managed to create a PR to my fork instead of to this repo for the other issue.)

In an effort to avoid creating too many merge conflicts for myself, I'm inclined to only try to change does not respond to No response received... as part of this issue, and once this (and maybe the other open ticket) are resolved, I can make a third ticket (or just a PR if people are ok w/ that) to shift queries to query in certain limited instances.

Anyway, I'm going to start by seeing if I can get this to work in a private fork today, and if I can, I'll submit a PR.

@matsduf
Copy link
Contributor

matsduf commented Jun 25, 2024

Please note that if the message is specified in the test case specification (e.g. specification for Basic02) then it should be updated there too, or more correctly, there first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Question Type: Incoming question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants