-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Open
Labels
A-http1Area: HTTP/1 specific.Area: HTTP/1 specific.C-refactorCategory: refactor. This would improve the clarity of internal code.Category: refactor. This would improve the clarity of internal code.E-easyEffort: easy. A task that would be a great starting point for a new contributor.Effort: easy. A task that would be a great starting point for a new contributor.
Description
As I mentioned in #2534, I believe we should adjust that internal assertion to not panic in release mode. The purpose of that check is to ensure that httparse
and http::HeaderName
agree on what is a valid value. We can generally assume they will agree, but it's still possible for bugs, so that's why we check it. I propose the following action:
Write a new macro, maybe bug!
(names are hard), that has the following behavior:
- When under
cfg(debug_assertions)
, continue to panic with the value it does currently. - When not, log a helpful message at the
error!
level, including that this is a bug and please report it. - As part of the non-panicking variant, the user of the macro should include an error value to return.
The suggestion to use a new error variant seems like a good idea too. Perhaps Parse::Internal
or Parse::Bug
.
let4be and bensadiku
Metadata
Metadata
Assignees
Labels
A-http1Area: HTTP/1 specific.Area: HTTP/1 specific.C-refactorCategory: refactor. This would improve the clarity of internal code.Category: refactor. This would improve the clarity of internal code.E-easyEffort: easy. A task that would be a great starting point for a new contributor.Effort: easy. A task that would be a great starting point for a new contributor.