cloudflare / cloudflare/pingora

Avoid panics by removing unwraps during HttpPeer and HttpHealthCheck initialization

Open
#921 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.