Changeset 222255 in webkit

Timestamp:

Sep 19, 2017 11:29:17 PM (7 years ago)

Author:

Carlos Garcia Campos

Message:

WebDriver: wrong response in case of errors
​https://bugs.webkit.org/show_bug.cgi?id=177127

Reviewed by Brian Burg.

I misunderstood the spec when I implemented this, so we either return a "value" key with the result in case of
success or the error object as the body in case of error. We should always add a "value" key to the body and set
it with either the result or the error object.

​https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-an-error

• WebDriverService.cpp:

(WebDriver::WebDriverService::sendResponse const):

Location:

trunk/Source/WebDriver

Files:

• ChangeLog (modified) (1 diff)
• WebDriverService.cpp (modified) (1 diff)

trunk/Source/WebDriver/ChangeLog

@@ +1 @@
+2017-09-19  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        WebDriver: wrong response in case of errors
+        https://bugs.webkit.org/show_bug.cgi?id=177127
+
+        Reviewed by Brian Burg.
+
+        I misunderstood the spec when I implemented this, so we either return a "value" key with the result in case of
+        success or the error object as the body in case of error. We should always add a "value" key to the body and set
+        it with either the result or the error object.
+
+        https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-an-error
+
+        * WebDriverService.cpp:
+        (WebDriver::WebDriverService::sendResponse const):
+
 2017-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebDriver/WebDriverService.cpp

@@ -249 +249 @@
 void WebDriverService::sendResponse(Function<void (HTTPRequestHandler::Response&&)>&& replyHandler, CommandResult&& result) const
 {
-    RefPtr<InspectorObject> responseObject;
+    // §6.3 Processing Model.
+    // https://w3c.github.io/webdriver/webdriver-spec.html#processing-model
+    RefPtr<InspectorValue> resultValue;
     if (result.isError()) {
-        responseObject = InspectorObject::create();
-        responseObject->setString(ASCIILiteral("error"), result.errorString());
-        responseObject->setString(ASCIILiteral("message"), result.errorMessage().value_or(emptyString()));
-        responseObject->setString(ASCIILiteral("stacktrace"), emptyString());
+        // When required to send an error.
+        // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-an-error
+        // Let body be a new JSON Object initialised with the following properties: "error", "message", "stacktrace".
+        auto errorObject = InspectorObject::create();
+        errorObject->setString(ASCIILiteral("error"), result.errorString());
+        errorObject->setString(ASCIILiteral("message"), result.errorMessage().value_or(emptyString()));
+        errorObject->setString(ASCIILiteral("stacktrace"), emptyString());
+        // If the error data dictionary contains any entries, set the "data" field on body to a new JSON Object populated with the dictionary.
         if (auto& additionalData = result.additionalErrorData())
-            responseObject->setObject(ASCIILiteral("data"), RefPtr<InspectorObject> { additionalData });
-    } else {
-        responseObject = InspectorObject::create();
-        auto resultValue = result.result();
-        responseObject->setValue(ASCIILiteral("value"), resultValue ? WTFMove(resultValue) : InspectorValue::null());
-    }
+            errorObject->setObject(ASCIILiteral("data"), RefPtr<InspectorObject> { additionalData });
+        // Send a response with status and body as arguments.
+        resultValue = WTFMove(errorObject);
+    } else if (auto value = result.result())
+        resultValue = WTFMove(value);
+    else
+        resultValue = InspectorValue::null();
+
+    // When required to send a response.
+    // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-send-a-response
+    RefPtr<InspectorObject> responseObject = InspectorObject::create();
+    responseObject->setValue(ASCIILiteral("value"), WTFMove(resultValue));
     replyHandler({ result.httpStatusCode(), responseObject->toJSONString().utf8(), ASCIILiteral("application/json; charset=utf-8") });
 }