-
Notifications
You must be signed in to change notification settings - Fork 70
1120 Trace URI on CRITICAL message Response #1323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1120
Are you sure you want to change the base?
Conversation
106e863
to
377de0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some example test statements, and also like Redfish Validator run
http/http2_connection.hpp
Outdated
@@ -207,7 +207,8 @@ class HTTP2Connection : | |||
|
|||
completeResponseFields(stream.accept, res); | |||
res.addHeader(boost::beast::http::field::date, getCachedDateStr()); | |||
res.preparePayload(); | |||
crow::Request& thisReq = *it->second.req; | |||
res.preparePayload(thisReq.url().path()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L204 already has this
Http2StreamData& stream = it->second;
So, these L210-L211 can be like
res.preparePayload(stream->url().path());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be
res.preparePayload(stream.req->url().path());
http/http_response.hpp
Outdated
@@ -186,7 +186,7 @@ struct Response | |||
return response.body().payloadSize(); | |||
} | |||
|
|||
void preparePayload() | |||
void preparePayload(std::string_view s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the argument can be more readable like
void preparePayload(std::string_view url)
http/http_response.hpp
Outdated
"allowed to have a body", | ||
logPtr(this)); | ||
"allowed to have a body \"{}\"", | ||
logPtr(this), s); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to specify url like this?
"allowed to have a body, for url {}",
Will want to go upstream here |
Trace URI on CRITICAL message Response content provided but code was no-content or not_modified. curl -k -H "X-Auth-Token: $bmc_token" -H "Content-Type: application/json" -X PATCH -d '{"LocationIndicatorActive":true}' https://$bmc/redfish/v1/Managers/bmc curl -k -H "Content-Type: application/json" -d '{"PowerRestorePolicy":"LastState"}' -X PATCH https://${bmc}/redfish/v1/Systems/system root@p10bmc:~# journalctl | grep 'Response content' Jun 19 16:51:33 p10bmc bmcwebd[1034]: [http_response.hpp:213] 0xb75210 Response content provided but code was no-content or not_modified, which aren't allowed t o have a body for url : "/redfish/v1/Managers/bmc" Jun 19 16:52:00 p10bmc bmcwebd[1034]: [http_response.hpp:213] 0xb75210 Response content provided but code was no-content or not_modified, which aren't allowed t o have a body for url : "/redfish/v1/Systems/system" Signed-off-by: Abiola Asojo <abiola.asojo@ibm.com>
377de0e
to
2d85598
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me.
(Please also upstream this and add Change-Id into this PR commit).
Trace URI on CRITICAL message Response content provided but code was no-content or not_modified.