Change parse assertion into an error log in release
Open
@bensadiku is already working on this.
Since May 12, 2021.
A-http1
C-refactor
E-easy
- Dominant language
- Rust
- Stars
- 16.3k
- Forks
- 1.8k
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 14
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.