Log H2 errors with the codes#5676
Merged
maskit merged 1 commit intoapache:masterfrom Jul 10, 2019
Merged
Conversation
maskit
force-pushed
the
h2_log_errors_with_code
branch
from
59e7288 to
ee1094a
Compare
zizhong
reviewed
Jun 27, 2019
| if (error.msg) { | ||
| Error("HTTP/2 connection error client_ip=%s session_id=%" PRId64 " stream_id=%u %s", client_ip, | ||
| ua_session->connection_id(), stream_id, error.msg); | ||
| Error("HTTP/2 connection error code=%d client_ip=%s session_id=%" PRId64 " stream_id=%u %s", static_cast<int>(error.code), |
Member
There was a problem hiding this comment.
Looks okay.
Is it better to convert the code to string to log, so user don't need to search the source to find out what the actual meaning?
Member
Author
There was a problem hiding this comment.
Well, I'm ok with using error names instead of codes. I'm not too worried about the log size but a line might get a bit too long, and making consistent length value might be easy to read (the maximum error code is 0x0d if we use hex).
Member
Author
There was a problem hiding this comment.
Now it prints error codes in hex decimal. Let me know if you still want it to be a string error name.
masaori335
reviewed
Jul 5, 2019
maskit
force-pushed
the
h2_log_errors_with_code
branch
from
37f5b33 to
f2313eb
Compare
Member
Author
|
[approve ci autest] |
1 similar comment
Member
Author
|
[approve ci autest] |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
H2 errors are logged only if they have error messages, and even with the messages, actual error codes are not logged.
We should log all errors with the codes at these places so that we can see what happened. If you don't want to log some particular errors for some reasons, you can skip these logging by passing
nullptras an error message (the default message is an empty string).If everything is working well, amount of logs should not increase.
I'd like to backport this change to older versions because I think this is essential information for troubleshooting.