webdriver: command: improve error handling of decoding request body as json
authorAndreas Tolfsen <ato@mozilla.com>
Thu, 20 Apr 2017 13:55:59 +0100
changeset 428145 a56d8bd25323d27ec5adcf35fd4987d96a54f5ba
parent 428144 d313a2a5bc7e20173965cfb20113068b97502419
child 428146 d89338a1bed23159868454b3c89d6b936729f5fa
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
webdriver: command: improve error handling of decoding request body as json This patch splits the decoding of the HTTP request body out to a separate function, decode_body, on WebDriverMessage. This is in line with get_session_id, and reduces somewhat the complexity of WebDriverMessage::from_http. It also handles rustc_serialize::json::ParserErrors individually, by making SyntaxError discernable from IoError. The SyntaxError error handler re-uses the stacktrace produced by the JSON parser when failing to decode the HTTP request body as JSON. This improves the readability of the returned error. Source-Repo: https://github.com/mozilla/webdriver-rust Source-Revision: d7fe400fa903b752b3c90c2c7e80ae9d5719e16b committer: jgraham <james@hoppipolla.co.uk>
testing/webdriver/src/command.rs
--- a/testing/webdriver/src/command.rs
+++ b/testing/webdriver/src/command.rs
@@ -1,14 +1,15 @@
 use capabilities::{SpecNewSessionParameters, LegacyNewSessionParameters,
                    CapabilitiesMatching, BrowserCapabilities, Capabilities};
 use common::{Date, Nullable, WebElement, FrameId, LocatorStrategy};
 use error::{WebDriverResult, WebDriverError, ErrorStatus};
 use httpapi::{Route, WebDriverExtensionRoute, VoidWebDriverExtensionRoute};
 use regex::Captures;
+use rustc_serialize::json;
 use rustc_serialize::json::{ToJson, Json};
 use std::collections::BTreeMap;
 use std::default::Default;
 
 #[derive(PartialEq)]
 pub enum WebDriverCommand<T: WebDriverExtensionCommand> {
     NewSession(NewSessionParameters),
     DeleteSession,
@@ -82,38 +83,34 @@ impl WebDriverExtensionCommand for VoidW
 }
 
 #[derive(PartialEq)]
 pub struct WebDriverMessage <U: WebDriverExtensionRoute=VoidWebDriverExtensionRoute> {
     pub session_id: Option<String>,
     pub command: WebDriverCommand<U::Command>,
 }
 
-impl <U: WebDriverExtensionRoute> WebDriverMessage<U> {
-    pub fn new(session_id: Option<String>, command: WebDriverCommand<U::Command>) -> WebDriverMessage<U> {
+impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
+    pub fn new(session_id: Option<String>,
+               command: WebDriverCommand<U::Command>)
+               -> WebDriverMessage<U> {
         WebDriverMessage {
             session_id: session_id,
             command: command,
         }
     }
 
-    pub fn from_http(match_type: Route<U>, params: &Captures, body: &str, requires_body: bool) -> WebDriverResult<WebDriverMessage<U>> {
+    pub fn from_http(match_type: Route<U>,
+                     params: &Captures,
+                     raw_body: &str,
+                     requires_body: bool)
+                     -> WebDriverResult<WebDriverMessage<U>> {
         let session_id = WebDriverMessage::<U>::get_session_id(params);
-        let body_data = if requires_body {
-            debug!("Got request body {}", body);
-            match Json::from_str(body) {
-                Ok(x @ Json::Object(_)) => x,
-                Ok(_) => return Err(WebDriverError::new(ErrorStatus::InvalidArgument,
-                                                        "Body was not a json object")),
-                Err(_) => return Err(WebDriverError::new(ErrorStatus::InvalidArgument,
-                                                         format!("Failed to decode request body as json: {}", body)))
-            }
-        } else {
-            Json::Null
-        };
+        let body_data = try!(WebDriverMessage::<U>::decode_body(raw_body, requires_body));
+
         let command = match match_type {
             Route::NewSession => {
                 let parameters: NewSessionParameters = try!(Parameters::from_json(&body_data));
                 WebDriverCommand::NewSession(parameters)
             },
             Route::DeleteSession => WebDriverCommand::DeleteSession,
             Route::Get => {
                 let parameters: GetParameters = try!(Parameters::from_json(&body_data));
@@ -338,16 +335,39 @@ impl <U: WebDriverExtensionRoute> WebDri
             }
         };
         Ok(WebDriverMessage::new(session_id, command))
     }
 
     fn get_session_id(params: &Captures) -> Option<String> {
         params.name("sessionId").map(|x| x.as_str().into())
     }
+
+    fn decode_body(body: &str, requires_body: bool) -> WebDriverResult<Json> {
+        if requires_body {
+            match Json::from_str(body) {
+                Ok(x @ Json::Object(_)) => Ok(x),
+                Ok(_) => {
+                    Err(WebDriverError::new(ErrorStatus::InvalidArgument,
+                                            "Body was not a JSON Object"))
+                }
+                Err(json::ParserError::SyntaxError(_, line, col)) => {
+                    let msg = format!("Failed to decode request as JSON: {}", body);
+                    let stack = format!("Syntax error at :{}:{}", line, col);
+                    Err(WebDriverError::new_with_stack(ErrorStatus::InvalidArgument, msg, stack))
+                }
+                Err(json::ParserError::IoError(e)) => {
+                    Err(WebDriverError::new(ErrorStatus::InvalidArgument,
+                                            format!("I/O error whilst decoding body: {}", e)))
+                }
+            }
+        } else {
+            Ok(Json::Null)
+        }
+    }
 }
 
 impl <U:WebDriverExtensionRoute> ToJson for WebDriverMessage<U> {
     fn to_json(&self) -> Json {
         let parameters = match self.command {
             WebDriverCommand::AcceptAlert |
             WebDriverCommand::CloseWindow |
             WebDriverCommand::ReleaseActions |