Skip to content

Conversation

@seniakalma
Copy link
Collaborator

Add health endpoint for Zebra node

@seniakalma seniakalma self-assigned this Aug 25, 2025
@seniakalma seniakalma marked this pull request as ready for review August 25, 2025 10:04
@seniakalma seniakalma requested a review from PaulLaux August 25, 2025 10:04
Copy link
Collaborator

@dmidem dmidem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, I just have a few suggestions - mainly around replacing some instances of .unwrap() with .expect("...") .


async fn handle_request(req: Request<Incoming>) -> Result<Response<String>, Infallible> {
// Check if the request is for the health endpoint
if req.uri().path() != "/health" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it additionally validate that it uses GET HTTP method only? Tracing endpoint validates that: https://github.com/QED-it/zebra/blob/zsa1/zebrad/src/components/tracing/endpoint.rs#L131

Copy link
Collaborator

@dmidem dmidem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of additional remarks are below.

Copy link
Collaborator

@dmidem dmidem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI fails now, to fix it, I suggest the following changes in Cargo.toml.

insta = { version = "1.40.0", features = ["json"] }

# zebra-rpc needs the preserve_order feature, it also makes test results more stable
serde_json = { version = "1.0.132", features = ["preserve_order"] }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider restoring serde_json under [dev-dependencies] (while keeping it in [dependencies] with optional = true), because cargo test without --features=health-endpoint currently fails with unresolved imports in tests that use serde_json directly, e.g.:

error[E0432]: unresolved import `serde_json`
   --> zebrad/tests/acceptance.rs:158:5
    |
158 | use serde_json::Value;
    |     ^^^^^^^^^^ help: a similar path exists: `jsonrpc_core::serde_json`

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V

@seniakalma
Copy link
Collaborator Author

Closed in favor of
#82

@seniakalma seniakalma closed this Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants