http::reply::status_type: Promote to namespace and make extensible struct #2884
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moves HTTP status codes to top-level type "status", and changes it from enum to struct. The former to be able to forward declare the type. The latter to be able to type-safely extend the type (i.e. define a typed constant in a different compilation unit, without having to modify a seastar enum.
added 2 commits
July 30, 2025 12:26
…tatus values Adds a dllink time bind function for adding custom constants to the http::status_type known names. The purpose is to allow both automatic nice formatting of received replies, but also ensure that a reply _sent_ using a custom status would format equivalently to pre-defined codes.
Add a few missing standard (RFC 9110) error codes:
407 Proxy Authentication Required
412 Precondition Failed
416 Range Not Satisfiable
421 Misdirected Request
Rename some string constant names to proper, modern names.
Code 302 is renamed from moved_temporarily to found, but
also aliased to the old constant name. String name is "Found".
This is shamelessly adapted from a patch by @nyh, but without
removing any codes (once using code adapts to custom constants
this can be done perhaps).
xemul
reviewed
Jul 31, 2025
| {reply::status_type::network_read_timeout, "598 Network Read Timeout"}, | ||
| {reply::status_type::network_connect_timeout, "599 Network Connect Timeout"}}; | ||
| static auto& status_strings() { | ||
| static std::unordered_map<status_type, std::string_view> status_strings = { |
There was a problem hiding this comment.
Why can't this map remain a global static file-local variable?
There was a problem hiding this comment.
Because we want to modify it at dllink time. If it was global, it would be subject to static init order fiasco. By placing it in a function we can ensure we init it before modifying.
There was a problem hiding this comment.
OK. Please add a code comment about it, because it's not immediately obvious
xemul
reviewed
Jul 31, 2025
| static constexpr status_init unsupported_media_type{415}; //!< unsupported_media_type | ||
| static constexpr status_init range_not_satisfiable{416}; //!< range_not_satisfiable | ||
| static constexpr status_init expectation_failed{417}; //!< expectation_failed | ||
| static constexpr status_init page_expired{419}; //!< page_expired |
There was a problem hiding this comment.
Can those non-standard codes be marked as [[deprecated(...)]] early, to be removed eventually, but also early?
There was a problem hiding this comment.
Probably a good idea.
xemul
reviewed
Aug 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs #884
Moves HTTP status codes to top-level type "status_type", and changes it from enum to struct.
The former to be able to forward declare the type.
The latter to be able to type-safely extend the type (i.e. define a typed constant in a different compilation unit, without having to modify a seastar enum).
"Extending" is done by simply constexpr-declaring your custom status in a code unit, then adding a dllink-time constant for static init registration, i.e:
Only valid in static init time.
Motivation:
1.) Using custom values for an enum type is oxymoronic. While it is workable to just cast int:s back and forth into the enum-values fields for status_type, it smells of bad practice. While replacing this with an admittedly weakly aliasing wrapper type is not much more type-safe, at least it is consistent.
2.) Some aspects of seastar level code relies (for some definition thereof) on translating status codes to strings, mainly pretty-printing a received reply (less important) and sending such a reply (see write_reply_to_connection). While a stringized value for the status is not mandatory, it is nice to be consistent, and get all info at all time without reinventing wheels.
Note: contains some haxxor to make the compile-time API as compatible as possible, i.e. include constexpr constants for all existing codes, as well as ensuring old code compiles (doing underlying_type_t stuff etc).