From 1ae4b57b5580601dfc36d4d333c48b01a8e1209c Mon Sep 17 00:00:00 2001 From: cobalt-github-releaser-bot <95661244+cobalt-github-releaser-bot@users.noreply.github.com> Date: Wed, 29 May 2024 19:03:31 -0700 Subject: [PATCH] Cherry pick PR #3378: Webdriver: Add extra production logging (#3381) Refer to the original PR: https://github.com/youtube/cobalt/pull/3378 Change logging from DLOG to LOG, so it's always available in QA builds. This should help troubleshooting Webdriver issues in future. b/343311752 Co-authored-by: Kaido Kert --- cobalt/webdriver/dispatcher.cc | 10 +++++++++- cobalt/webdriver/protocol/cookie.cc | 20 ++++++++++---------- cobalt/webdriver/protocol/proxy.cc | 2 +- cobalt/webdriver/script_executor_params.cc | 2 +- cobalt/webdriver/server.cc | 9 ++++++--- cobalt/webdriver/window_driver.cc | 2 +- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/cobalt/webdriver/dispatcher.cc b/cobalt/webdriver/dispatcher.cc index f10bdc7fcda8..f7dac62bd4e7 100644 --- a/cobalt/webdriver/dispatcher.cc +++ b/cobalt/webdriver/dispatcher.cc @@ -48,6 +48,7 @@ class CommandResultHandlerImpl if (status_code == protocol::Response::kSuccess) { response_handler_->Success(std::move(response)); } else { + LOG(ERROR) << "SendResult status code:" << status_code; response_handler_->FailedCommand(std::move(response)); } } @@ -58,6 +59,8 @@ class CommandResultHandlerImpl if (status_code == protocol::Response::kSuccess) { response_handler_->SuccessData(content_type, data, len); } else { + LOG(ERROR) << "SendResultWithContentType :" << content_type + << "status code:" << status_code << " [" << data << "]"; std::unique_ptr response = protocol::Response::CreateResponse(base::nullopt, status_code, NULL); response_handler_->FailedCommand(std::move(response)); @@ -68,9 +71,13 @@ class CommandResultHandlerImpl const std::string& error_string) override { switch (error) { case kInvalidParameters: + LOG(ERROR) << "SendInvalidRequestResponse::kInvalidParameters (" + << error_string << ")"; response_handler_->MissingCommandParameters(error_string); break; case kInvalidPathVariable: + LOG(ERROR) << "SendInvalidRequestResponse::kInvalidPathVariabl (" + << error_string << ")"; response_handler_->VariableResourceNotFound(error_string); break; } @@ -180,7 +187,8 @@ WebDriverDispatcher::CommandMapping* WebDriverDispatcher::GetMappingForPath( typedef std::vector::const_reverse_iterator MismatchResult; typedef std::pair MismatchResultPair; typedef std::pair EqualRangeResultPair; + CommandMappingLookup::iterator> + EqualRangeResultPair; PathComponentsAreEqualPredicate predicate(strategy == kMatchExact); diff --git a/cobalt/webdriver/protocol/cookie.cc b/cobalt/webdriver/protocol/cookie.cc index 59ea0625d61e..c74d62e1157d 100644 --- a/cobalt/webdriver/protocol/cookie.cc +++ b/cobalt/webdriver/protocol/cookie.cc @@ -12,10 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include - #include "cobalt/webdriver/protocol/cookie.h" +#include + #include "base/strings/string_split.h" #include "base/strings/stringprintf.h" @@ -49,13 +49,13 @@ base::Optional Cookie::FromValue(const base::Value* value) { // error, but the current implementation will return "invalid parameter". const base::DictionaryValue* dictionary_value; if (!value->GetAsDictionary(&dictionary_value)) { - DLOG(INFO) << "Parameter is not a dictionary."; + LOG(ERROR) << "Parameter is not a dictionary."; return base::nullopt; } const base::DictionaryValue* cookie_dictionary_value; if (!dictionary_value->GetDictionary(kCookieKey, &cookie_dictionary_value)) { - DLOG(INFO) << base::StringPrintf("Value of key [%s] is not a JSON object.", + LOG(ERROR) << base::StringPrintf("Value of key [%s] is not a JSON object.", kCookieKey); return base::nullopt; } @@ -65,7 +65,7 @@ base::Optional Cookie::FromValue(const base::Value* value) { // Name and value are required. if (!cookie_dictionary_value->GetString(kNameKey, &cookie_name) || !cookie_dictionary_value->GetString(kValueKey, &cookie_value)) { - DLOG(INFO) << base::StringPrintf( + LOG(ERROR) << base::StringPrintf( "cookie.%s or cookie.%s either does not exist or is not a string", kNameKey, kValueKey); return base::nullopt; @@ -78,7 +78,7 @@ base::Optional Cookie::FromValue(const base::Value* value) { if (cookie_dictionary_value->GetString(kDomainKey, &string_value)) { new_cookie.domain_ = string_value; } else { - DLOG(INFO) << base::StringPrintf("cookie.%s is not a string", kDomainKey); + LOG(ERROR) << base::StringPrintf("cookie.%s is not a string", kDomainKey); return base::nullopt; } } @@ -86,7 +86,7 @@ base::Optional Cookie::FromValue(const base::Value* value) { if (cookie_dictionary_value->GetString(kPathKey, &string_value)) { new_cookie.path_ = string_value; } else { - DLOG(INFO) << base::StringPrintf("cookie.%s is not a string", kPathKey); + LOG(ERROR) << base::StringPrintf("cookie.%s is not a string", kPathKey); return base::nullopt; } } @@ -96,7 +96,7 @@ base::Optional Cookie::FromValue(const base::Value* value) { if (cookie_dictionary_value->GetBoolean(kSecureKey, &bool_value)) { new_cookie.secure_ = bool_value; } else { - DLOG(INFO) << base::StringPrintf("cookie.%s is not a boolean", + LOG(ERROR) << base::StringPrintf("cookie.%s is not a boolean", kSecureKey); return base::nullopt; } @@ -105,7 +105,7 @@ base::Optional Cookie::FromValue(const base::Value* value) { if (cookie_dictionary_value->GetBoolean(kHttpOnlyKey, &bool_value)) { new_cookie.http_only_ = bool_value; } else { - DLOG(INFO) << base::StringPrintf("cookie.%s is not a boolean", + LOG(ERROR) << base::StringPrintf("cookie.%s is not a boolean", kHttpOnlyKey); return base::nullopt; } @@ -118,7 +118,7 @@ base::Optional Cookie::FromValue(const base::Value* value) { base::TimeDelta::FromSeconds(timestamp_value); new_cookie.expiry_time_ = base::Time::UnixEpoch() + seconds_since_epoch; } else { - DLOG(INFO) << base::StringPrintf("cookie.%s is not an integer", + LOG(ERROR) << base::StringPrintf("cookie.%s is not an integer", kExpiryKey); return base::nullopt; } diff --git a/cobalt/webdriver/protocol/proxy.cc b/cobalt/webdriver/protocol/proxy.cc index 86315c4447e1..fa75de3504ef 100644 --- a/cobalt/webdriver/protocol/proxy.cc +++ b/cobalt/webdriver/protocol/proxy.cc @@ -59,7 +59,7 @@ base::Optional Proxy::FromValue(const base::Value* value) { } } else { // Only manual proxy type is supported. - DLOG(INFO) << "Unsupported proxy type: " << proxy_type_string; + LOG(ERROR) << "Unsupported proxy type: " << proxy_type_string; } return base::nullopt; } diff --git a/cobalt/webdriver/script_executor_params.cc b/cobalt/webdriver/script_executor_params.cc index f6a865692679..18c7720a0aee 100644 --- a/cobalt/webdriver/script_executor_params.cc +++ b/cobalt/webdriver/script_executor_params.cc @@ -45,7 +45,7 @@ ScriptExecutorParams::GCPreventedParams ScriptExecutorParams::Create( if (!global_environment->EvaluateScript(function_source, params.get(), ¶ms->function_object_)) { - DLOG(ERROR) << "Failed to create Function object"; + LOG(ERROR) << "Failed to create Function object"; } return {params, global_environment.get()}; } diff --git a/cobalt/webdriver/server.cc b/cobalt/webdriver/server.cc index 06093eb0850f..461d75d91796 100644 --- a/cobalt/webdriver/server.cc +++ b/cobalt/webdriver/server.cc @@ -111,14 +111,14 @@ class ResponseHandlerImpl : public WebDriverServer::ResponseHandler { // The command request is not mapped to anything. void UnknownCommand(const std::string& path) override { - LOG(INFO) << "Unknown command: " << path; + LOG(WARNING) << "Unknown command: " << path; SendInternal(net::HTTP_NOT_FOUND, "Unknown command", kTextPlainContentType); } // The command request is mapped to a valid command, but this WebDriver // implementation has not implemented it. void UnimplementedCommand(const std::string& path) override { - LOG(INFO) << "Unimplemented command: " << path; + LOG(ERROR) << "Unimplemented command: " << path; SendInternal(net::HTTP_NOT_IMPLEMENTED, "Unimplemented command", kTextPlainContentType); } @@ -126,6 +126,7 @@ class ResponseHandlerImpl : public WebDriverServer::ResponseHandler { // The request maps to a valid command, but the variable part of the path // does not map to a valid instance. void VariableResourceNotFound(const std::string& variable_name) override { + LOG(ERROR) << "VariableResourceNotFound : " << variable_name; SendInternal(net::HTTP_NOT_FOUND, "Unknown variable resource: " + variable_name, kTextPlainContentType); @@ -143,6 +144,7 @@ class ResponseHandlerImpl : public WebDriverServer::ResponseHandler { net::HttpServerResponseInfo response_info; response_info.AddHeader("Allow", base::JoinString(allowed_method_strings, ", ")); + LOG(ERROR) << "InvalidCommandMethod (" << requested_method << ")"; SendInternal(net::HTTP_METHOD_NOT_ALLOWED, "Invalid method: " + HttpMethodToString(requested_method), kTextPlainContentType, response_info); @@ -151,6 +153,7 @@ class ResponseHandlerImpl : public WebDriverServer::ResponseHandler { // The POST command's JSON request body does not contain the required // parameters. void MissingCommandParameters(const std::string& message) override { + LOG(ERROR) << "MissingCommandParameters (" << message << ")"; SendInternal(net::HTTP_BAD_REQUEST, message, kTextPlainContentType); } @@ -225,7 +228,7 @@ void WebDriverServer::OnHttpRequest(int connection_id, path.resize(query_position); } - DLOG(INFO) << "Got request: " << path; + LOG(INFO) << "Got request: " << path; // Create a new ResponseHandler that will send a response to this connection. std::unique_ptr response_handler( new ResponseHandlerImpl(server_.get(), connection_id)); diff --git a/cobalt/webdriver/window_driver.cc b/cobalt/webdriver/window_driver.cc index 43f213af08d1..e762cba97716 100644 --- a/cobalt/webdriver/window_driver.cc +++ b/cobalt/webdriver/window_driver.cc @@ -474,7 +474,7 @@ util::CommandResult WindowDriver::ExecuteScriptInternal( scoped_refptr script_executor = ScriptExecutor::Create(this, global_environment); if (!script_executor) { - DLOG(INFO) << "Failed to create ScriptExecutor."; + LOG(ERROR) << "Failed to create ScriptExecutor."; return CommandResult(protocol::Response::kUnknownError); } script_executor_ = base::AsWeakPtr(script_executor.get());