Skip to content

Commit

Permalink
Webdriver: Add extra production logging
Browse files Browse the repository at this point in the history 
Change logging from DLOG to LOG, so it's always available in QA
builds. This should help troubleshooting Webdriver issues in
future.

Cherry-pick to 24LTS from #3378

b/343311752
  • Loading branch information
@kaidokert
kaidokert committed May 30, 2024
1 parent 145a536 commit d05171d
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 17 deletions.
10 changes: 9 additions & 1 deletion cobalt/webdriver/dispatcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand All @@ -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));
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);

Expand Down
20 changes: 10 additions & 10 deletions cobalt/webdriver/protocol/cookie.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand All @@ -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;
}
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion cobalt/webdriver/protocol/proxy.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion cobalt/webdriver/script_executor_params.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
}
Expand Down
9 changes: 6 additions & 3 deletions cobalt/webdriver/server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion cobalt/webdriver/window_driver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit d05171d

Please sign in to comment.