Cherry pick PR #3378: Webdriver: Add extra production logging (#3381)

Refer to the original PR: #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 <[email protected]>
youtube · May 30, 2024 · 1ae4b57 · 1ae4b57
1 parent 032bff3
commit 1ae4b57
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 17 deletions.
diff --git 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<base::Value> 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<std::string>::const_reverse_iterator MismatchResult;
   typedef std::pair<MismatchResult, MismatchResult> MismatchResultPair;
   typedef std::pair<CommandMappingLookup::iterator,
-                    CommandMappingLookup::iterator> EqualRangeResultPair;
+                    CommandMappingLookup::iterator>
+      EqualRangeResultPair;
 
   PathComponentsAreEqualPredicate predicate(strategy == kMatchExact);
 

diff --git 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 <memory>
-
 #include "cobalt/webdriver/protocol/cookie.h"
 
+#include <memory>
+
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 
@@ -49,13 +49,13 @@ base::Optional<Cookie> 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> 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,15 +78,15 @@ base::Optional<Cookie> 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;
     }
   }
   if (cookie_dictionary_value->HasKey(kPathKey)) {
     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> 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> 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> 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
@@ -59,7 +59,7 @@ base::Optional<Proxy> 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
@@ -45,7 +45,7 @@ ScriptExecutorParams::GCPreventedParams ScriptExecutorParams::Create(
   if (!global_environment->EvaluateScript(function_source, params.get(),
 
                                           &params->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
@@ -111,21 +111,22 @@ 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);
   }
 
   // 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<ResponseHandler> response_handler(
       new ResponseHandlerImpl(server_.get(), connection_id));

diff --git a/cobalt/webdriver/window_driver.cc b/cobalt/webdriver/window_driver.cc
@@ -474,7 +474,7 @@ util::CommandResult<protocol::ScriptResult> WindowDriver::ExecuteScriptInternal(
     scoped_refptr<ScriptExecutor> 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());