WIP: Simple NgxResult type and related refactoring in Pool, Request, and variable handlers.

[ensh63]

Proposed changes

• Simplest NgxResult is defined. Error type inside to be discussed.
• Pool interface is redefined to use NgxResult wherever possible. Some dangerous methods are removed.
• Macros for request handler and variable getter/setter are redefined to use NgxResult inside.
• Alternative Request handler definition via special trait is added.

This refactoring is subject to discuss. For instance, dumb error definition may be not enough in some cases.

[Copilot]

Pull Request Overview

This PR introduces a unified NgxResult type for error handling across the nginx Rust wrapper codebase and refactors the Pool, Request, and variable handler interfaces to use it. It replaces the previous pattern of returning Status directly with a Result<T, NgxError> pattern to provide more ergonomic error handling in Rust.

Key changes:

• Introduces NgxResult type with a simple NgxError wrapper
• Refactors Pool interface to use Result and NonNull for safer memory allocation
• Updates request handlers, variable getters/setters to use NgxResult
• Adds alternative RequestHandler trait for cleaner request handler registration

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/core/status.rs	Defines NgxError and NgxResult types with conversions from Status
src/core/pool.rs	Refactors Pool methods to return Result<NonNull<T>, AllocError>
src/http/request.rs	Updates macros and methods to use NgxResult, adds RequestHandler trait
src/http/module.rs	Adds request handler registration support to HttpModule trait
src/http/status.rs	Adds conversion from HTTPStatus to NgxResult, fixes typo
src/http/upstream.rs	Updates upstream init peer macro to use NgxResult
nginx-sys/src/http.rs	Adds NgxHttpPhases enum for HTTP phase definitions
examples/*.rs	Updates example code to use new NgxResult pattern

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The pool.as_ptr() method call suggests accessing the raw pointer from a Pool, but this may not be the intended API usage. Consider using the Pool's proper allocation methods instead of accessing the raw pointer directly.

[ensh63]

This correction makes sense, but this is a possible future improvement for string module.

[pchickey]

alloc_with_cleanup and allocate_with_cleanup are very nearly the same. Could we get rid of alloc_with_cleanup to encourage use of the more rust-idiomatic interface? If we can't, can we at least implement it in terms of allocate_with_cleanup?

[ensh63]

The only reason to keep these two variants are in create_..._conf() methods in HttpModule (see here, for example). It is possible to use allocate_with_cleanup() everywhere, but in this case there would be double type conversion in config creation methods. maybe it is OK, but I am not sure.

[pchickey]

If we just have to change the use in the default method implementations there, then it should be fine to fix it there and abolish alloc_with_cleanup completely. I haven't dug into exactly why HttpModule trait uses *mut c_void everywhere instead of -> Result<NonNull<T>, AllocError> in those methods, since the comment says the caller has to handle null pointers and that signature is the right way to encode that invariant in Rust.

[ensh63]

These methods in HttpModule implement direct C callbacks, so *mut c_void is an expected return type. That's why double conversion would be needed somewhere if use allocator with Result<NonNull<T>, AllocError> instead. These type conversions should be zero-cost at runtime, but extra code without any real meaning looks strange for me. But I will try to overlook these parts again.