cloudflare / cloudflare/pingora
Avoid panics by removing unwraps during HttpPeer and HttpHealthCheck initialization
- Dominant language
- Rust
- Stars
- 27.4k
- Forks
- 1.7k
- Avg merge
- 6h 22m
- Merged PRs (30d)
- 3
Description
Hello,
### Summary
I would like to propose an improvement to enhance the stability of Pingora's upstream peer and health check modules by removing potential runtime panics during initialization.
---
### The Problem
Several constructors in the codebase rely on `.unwrap()`, which can cause a worker process to panic unexpectedly at runtime if an error occurs:
1. **`HttpPeer`**:
* `HttpPeer::new()` and `HttpPeer::new_mtls()` call `.unwrap()` on the result of `to_socket_addrs()` and on the subsequent iterator (`.next()`). If DNS resolution fails or returns no addresses, the process panics.
2. **`HttpHealthCheck`**:
* `HttpHealthCheck::new()` and `HttpHealthCheck::new_custom()` use `.unwrap()` during request header construction (`RequestHeader::build`) and peer template initialization.
---
### Proposed Solution
* Refactor these initializers to safely propagate errors using the `?` operator instead of panicking.
* Change the return signature of `HttpPeer::new`, `HttpPeer::new_mtls`, `HttpHealthCheck::new`, and `HttpHealthCheck::new_custom` to return `Result` instead of `Self`.
* Update the associated test suites to handle the new `Result` returns properly.
---
I have prepared a Pull Request containing these modifications and updated the relevant tests to ensure the codebase remains stable and compiles cleanly.
_____
## Pingora info
**Pingora version**: 0.8.1
## Steps to reproduce
A panic occurs if the DNS server is unreachable.
Contributor guide
Research direction
Locate the definitions of HttpPeer::new, HttpPeer::new_mtls, HttpHealthCheck::new, and HttpHealthCheck::new_custom, then inspect their associated test suites and unwrap sites. The work is done when initialization errors are returned as Results and all affected callers and tests handle those returns without panicking.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100