[PR #3252] [MERGED] Catalog: Send error responses in more edge cases #3681

New issue

Closed

opened 2026-03-16 11:56:57 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:56:57 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/3252
Author: @divergentdave
Created: 9/3/2025
Status: ✅ Merged
Merged: 9/3/2025
Merged by: @divergentdave

Base: main ← Head: david/catalog-update-error-responses

📝 Commits (6)

57b9263 Change one more error message
f55dc1b Factor out helper for sending error responses
6d0962f Use helper for BADVERS as well
6b8aa10 Send error responses in various edge cases
5cc00b8 Add tests for empty zone handler chain in catalog
632401f Store Edns in MessageResponse as a reference

📊 Changes

6 files changed (+312 additions, -117 deletions)

View changed files

📝 crates/server/src/zone_handler/catalog.rs (+213 -104)
📝 crates/server/src/zone_handler/message_response.rs (+9 -9)
📝 tests/integration-tests/src/mock_request_handler.rs (+1 -1)
📝 tests/integration-tests/tests/integration/catalog_tests.rs (+86 -0)
📝 tests/integration-tests/tests/integration/dnssec_client_handle_tests.rs (+1 -1)
📝 tests/integration-tests/tests/integration/sqlite_zone_handler_tests.rs (+2 -2)

📄 Description

While I was working on an UPDATE test, I noticed another case where we were failing to send a response in certain error conditions (leading to a test hang). This is related to #2572. I went through Catalog::handle_request() and its call tree to send error responses in more cases; we should have complete coverage now. Error responses were primarily missing in Catalog::update(), in fall-through code where no zone handler in a chain handled a request, and during response signing where ? was used to return early. I changed the return type of some functions or methods from a Result to a ResponseInfo, which helped flush out the latter. I also changed MessageResponse so that it only holds a reference to the response's Edns struct, not ownership of it, in order to eliminate some unnecessary clones.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/3252 **Author:** [@divergentdave](https://github.com/divergentdave) **Created:** 9/3/2025 **Status:** ✅ Merged **Merged:** 9/3/2025 **Merged by:** [@divergentdave](https://github.com/divergentdave) **Base:** `main` ← **Head:** `david/catalog-update-error-responses` --- ### 📝 Commits (6) - [`57b9263`](https://github.com/hickory-dns/hickory-dns/commit/57b926361a5727cf2c96d45bc3fa9cd31cb2ff36) Change one more error message - [`f55dc1b`](https://github.com/hickory-dns/hickory-dns/commit/f55dc1bc494cb6bb07114bec439f4c38162d98d3) Factor out helper for sending error responses - [`6d0962f`](https://github.com/hickory-dns/hickory-dns/commit/6d0962feab863e81699ba209c53588e7612c0d79) Use helper for BADVERS as well - [`6b8aa10`](https://github.com/hickory-dns/hickory-dns/commit/6b8aa10509f723f6a49235dbe129d883d074c22f) Send error responses in various edge cases - [`5cc00b8`](https://github.com/hickory-dns/hickory-dns/commit/5cc00b884ac64e1a43da9e89a155dfc2c7fb07bf) Add tests for empty zone handler chain in catalog - [`632401f`](https://github.com/hickory-dns/hickory-dns/commit/632401f177920c6a8fa9b4290cd70b9ee7996013) Store Edns in MessageResponse as a reference ### 📊 Changes **6 files changed** (+312 additions, -117 deletions) <details> <summary>View changed files</summary> 📝 `crates/server/src/zone_handler/catalog.rs` (+213 -104) 📝 `crates/server/src/zone_handler/message_response.rs` (+9 -9) 📝 `tests/integration-tests/src/mock_request_handler.rs` (+1 -1) 📝 `tests/integration-tests/tests/integration/catalog_tests.rs` (+86 -0) 📝 `tests/integration-tests/tests/integration/dnssec_client_handle_tests.rs` (+1 -1) 📝 `tests/integration-tests/tests/integration/sqlite_zone_handler_tests.rs` (+2 -2) </details> ### 📄 Description While I was working on an UPDATE test, I noticed another case where we were failing to send a response in certain error conditions (leading to a test hang). This is related to #2572. I went through `Catalog::handle_request()` and its call tree to send error responses in more cases; we should have complete coverage now. Error responses were primarily missing in `Catalog::update()`, in fall-through code where no zone handler in a chain handled a request, and during response signing where `?` was used to return early. I changed the return type of some functions or methods from a `Result` to a `ResponseInfo`, which helped flush out the latter. I also changed `MessageResponse` so that it only holds a reference to the response's `Edns` struct, not ownership of it, in order to eliminate some unnecessary clones. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>