search

zonemaster / zonemaster-engine

The Zonemaster Engine - part of the Zonemaster project
Other
33 stars 31 forks source link

Reword `does not respond` messages #1354

Open jsoref opened 1 month ago

jsoref commented 1 month ago

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

https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Basic.pm#L252

see #1353 for name server vs nameserver

https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Connectivity.pm#L193 https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Connectivity.pm#L197 https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Connectivity.pm#L201 https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Connectivity.pm#L237 https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Connectivity.pm#L241 https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Connectivity.pm#L245 https://github.com/zonemaster/zonemaster-engine/blob/17688f7995e16dd9f5a4bec2a1437fc7804b31f0/lib/Zonemaster/Engine/Test/Zone.pm#L405

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:

name server could be:

matsduf commented 1 month ago

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

tgreenx commented 3 weeks ago

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)

matsduf commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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.