[chromedriver] Translate DevTools InvalidParams into InvalidArgument

When DevTools reports an error with code InvalidParams, translate it
into an InvalidArgument error instead of UnknownError. This lets us
perform certain checks on the devtools side that are not possible on the
chromedriver side and return InvalidArgument when they fail without
comparing against the error description on chromedriver. Additionally,
we can avoid duplicating certain parameter checking on the chromedriver
side.

This patch also adds tests for the ParseInspectorError function.

Bug: chromedriver:3025
Change-Id: I4a01136c87a53a1c5b2baac20c1ea65a60699d4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1706618
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Auto-Submit: Nina Satragno <nsatragno@chromium.org>
Reviewed-by: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678397}

chrome/test/chromedriver/chrome/devtools_client_impl.cc

@@ -27,25 +27,7 @@ const char kInspectorDefaultContextError[] =
 const char kInspectorContextError[] =
     "Cannot find execution context with given id";
 const char kInspectorInvalidURL[] = "Cannot navigate to invalid URL";
-
-Status ParseInspectorError(const std::string& error_json) {
-  std::unique_ptr<base::Value> error =
-      base::JSONReader::ReadDeprecated(error_json);
-  base::DictionaryValue* error_dict;
-  if (!error || !error->GetAsDictionary(&error_dict))
-    return Status(kUnknownError, "inspector error with no error message");
-  std::string error_message;
-  bool error_found = error_dict->GetString("message", &error_message);
-  if (error_found) {
-    if (error_message == kInspectorDefaultContextError ||
-        error_message == kInspectorContextError) {
-      return Status(kNoSuchExecutionContext);
-    } else if (error_message == kInspectorInvalidURL) {
-      return Status(kInvalidArgument);
-    }
-  }
-  return Status(kUnknownError, "unhandled inspector error: " + error_json);
-}
+static constexpr int kInvalidParamsInspectorCode = -32602;
 
 class ScopedIncrementer {
  public:
@@ -358,7 +340,7 @@ Status DevToolsClientImpl::SendCommandInternal(
       CHECK_EQ(response_info->state, kReceived);
       internal::InspectorCommandResponse& response = response_info->response;
       if (!response.result)
-        return ParseInspectorError(response.error);
+        return internal::ParseInspectorError(response.error);
       *result = std::move(response.result);
     }
   } else {
@@ -642,4 +624,26 @@ bool ParseInspectorMessage(
   return false;
 }
 
+Status ParseInspectorError(const std::string& error_json) {
+  std::unique_ptr<base::Value> error =
+      base::JSONReader::ReadDeprecated(error_json);
+  base::DictionaryValue* error_dict;
+  if (!error || !error->GetAsDictionary(&error_dict))
+    return Status(kUnknownError, "inspector error with no error message");
+  std::string error_message;
+  bool error_found = error_dict->GetString("message", &error_message);
+  if (error_found) {
+    if (error_message == kInspectorDefaultContextError ||
+        error_message == kInspectorContextError) {
+      return Status(kNoSuchExecutionContext);
+    } else if (error_message == kInspectorInvalidURL) {
+      return Status(kInvalidArgument);
+    }
+    base::Optional<int> error_code = error_dict->FindIntPath("code");
+    if (error_code == kInvalidParamsInspectorCode)
+      return Status(kInvalidArgument, error_message);
+  }
+  return Status(kUnknownError, "unhandled inspector error: " + error_json);
+}
+
 }  // namespace internal

chrome/test/chromedriver/chrome/devtools_client_impl.h

@@ -193,6 +193,8 @@ bool ParseInspectorMessage(
     InspectorEvent* event,
     InspectorCommandResponse* command_response);
 
+Status ParseInspectorError(const std::string& error_json);
+
 }  // namespace internal
 
 #endif  // CHROME_TEST_CHROMEDRIVER_CHROME_DEVTOOLS_CLIENT_IMPL_H_

chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc

@@ -574,6 +574,34 @@ TEST(ParseInspectorMessage, Command) {
   ASSERT_EQ(1, key);
 }
 
+TEST(ParseInspectorError, EmptyError) {
+  Status status = internal::ParseInspectorError("");
+  ASSERT_EQ(kUnknownError, status.code());
+  ASSERT_EQ("unknown error: inspector error with no error message",
+            status.message());
+}
+
+TEST(ParseInspectorError, InvalidUrlError) {
+  Status status = internal::ParseInspectorError(
+      "{\"message\": \"Cannot navigate to invalid URL\"}");
+  ASSERT_EQ(kInvalidArgument, status.code());
+}
+
+TEST(ParseInspectorError, InvalidArgumentCode) {
+  Status status = internal::ParseInspectorError(
+      "{\"code\": -32602, \"message\": \"Error description\"}");
+  ASSERT_EQ(kInvalidArgument, status.code());
+  ASSERT_EQ("invalid argument: Error description", status.message());
+}
+
+TEST(ParseInspectorError, UnknownError) {
+  const std::string error("{\"code\": 10, \"message\": \"Error description\"}");
+  Status status = internal::ParseInspectorError(error);
+  ASSERT_EQ(kUnknownError, status.code());
+  ASSERT_EQ("unknown error: unhandled inspector error: " + error,
+            status.message());
+}
+
 TEST_F(DevToolsClientImplTest, HandleEventsUntil) {
   MockListener listener;
   SyncWebSocketFactory factory =