mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1396819 - Improve error messages from capabilities matching. r=whimboo 

This patch somewhat marginally improves error messages returned
during capabilities negotiation.

MozReview-Commit-ID: IHTRk7Rl4ZU

--HG--
extra : rebase_source : 0fdca046ad69f732d70d5a23145578697cc2fb61
This commit is contained in:
Andreas Tolfsen 2018-02-14 15:43:56 +00:00
Родитель d5db540205
Коммит e963826d1c
2 изменённых файлов: 137 добавлений и 100 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

235
testing/webdriver/src/capabilities.rs
Просмотреть файл

@ -87,13 +87,13 @@ impl SpecNewSessionParameters {
match &**key { match &**key {
"acceptInsecureCerts" => if !value.is_boolean() { "acceptInsecureCerts" => if !value.is_boolean() {
return Err(WebDriverError::new(ErrorStatus::InvalidArgument, return Err(WebDriverError::new(ErrorStatus::InvalidArgument,
"acceptInsecureCerts is not a boolean")) format!("acceptInsecureCerts is not boolean: {}", value)))
}, },
x @ "browserName" | x @ "browserName" |
x @ "browserVersion" | x @ "browserVersion" |
x @ "platformName" => if !value.is_string() { x @ "platformName" => if !value.is_string() {
return Err(WebDriverError::new(ErrorStatus::InvalidArgument, return Err(WebDriverError::new(ErrorStatus::InvalidArgument,
format!("{} is not a string", x))) format!("{} is not a string: {}", x, value)))
}, },
"pageLoadStrategy" => { "pageLoadStrategy" => {
try!(SpecNewSessionParameters::validate_page_load_strategy(value)) try!(SpecNewSessionParameters::validate_page_load_strategy(value))
@ -110,7 +110,7 @@ impl SpecNewSessionParameters {
x => { x => {
if !x.contains(":") { if !x.contains(":") {
return Err(WebDriverError::new(ErrorStatus::InvalidArgument, return Err(WebDriverError::new(ErrorStatus::InvalidArgument,
format!("{} is not the name of a known capability or a valid extension capability", x))) format!("{} is not the name of a known capability or extension capability", x)))
} else { } else {
try!(browser_capabilities.validate_custom(x, value)); try!(browser_capabilities.validate_custom(x, value));
} }
@ -130,7 +130,7 @@ impl SpecNewSessionParameters {
x => { x => {
return Err(WebDriverError::new( return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("\"{}\" is not a valid page load strategy", x))) format!("Invalid page load strategy: {}", x)))
} }
} }
} }
@ -144,6 +144,7 @@ impl SpecNewSessionParameters {
let obj = try_opt!(proxy_value.as_object(), let obj = try_opt!(proxy_value.as_object(),
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"proxy is not an object"); "proxy is not an object");
for (key, value) in obj.iter() { for (key, value) in obj.iter() {
match &**key { match &**key {
"proxyType" => match value.as_string() { "proxyType" => match value.as_string() {
@ -154,36 +155,42 @@ impl SpecNewSessionParameters {
Some("manual") => {}, Some("manual") => {},
Some(x) => return Err(WebDriverError::new( Some(x) => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not a valid proxyType value", x))), format!("Invalid proxyType value: {}", x))),
None => return Err(WebDriverError::new( None => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"proxyType value is not a string")), format!("proxyType is not a string: {}", value))),
}, },
"proxyAutoconfigUrl" => match value.as_string() { "proxyAutoconfigUrl" => match value.as_string() {
Some(x) => { Some(x) => {
try!(Url::parse(x).or(Err(WebDriverError::new( Url::parse(x).or(Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"proxyAutoconfigUrl is not a valid url")))); format!("proxyAutoconfigUrl is not a valid URL: {}", x))))?;
}, },
None => return Err(WebDriverError::new( None => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"proxyAutoconfigUrl is not a string" "proxyAutoconfigUrl is not a string"
)) ))
}, },
"ftpProxy" => try!(SpecNewSessionParameters::validate_host(value)),
"httpProxy" => try!(SpecNewSessionParameters::validate_host(value)), "ftpProxy" => SpecNewSessionParameters::validate_host(value, "ftpProxy")?,
"noProxy" => try!(SpecNewSessionParameters::validate_no_proxy(value)), "httpProxy" => SpecNewSessionParameters::validate_host(value, "httpProxy")?,
"sslProxy" => try!(SpecNewSessionParameters::validate_host(value)), "noProxy" => SpecNewSessionParameters::validate_no_proxy(value)?,
"socksProxy" => try!(SpecNewSessionParameters::validate_host(value)), "sslProxy" => SpecNewSessionParameters::validate_host(value, "sslProxy")?,
"socksProxy" => SpecNewSessionParameters::validate_host(value, "socksProxy")?,
"socksVersion" => if !value.is_number() { "socksVersion" => if !value.is_number() {
return Err(WebDriverError::new(ErrorStatus::InvalidArgument, return Err(WebDriverError::new(
"socksVersion is not a number")) ErrorStatus::InvalidArgument,
format!("socksVersion is not a number: {}", value)
))
}, },
x => return Err(WebDriverError::new( x => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not a valid proxy configuration capability", x))) format!("Invalid proxy configuration entry: {}", x)))
} }
} }
Ok(()) Ok(())
} }
@ -195,14 +202,14 @@ impl SpecNewSessionParameters {
Some(_) => {}, Some(_) => {},
None => return Err(WebDriverError::new( None => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not a string", host) format!("noProxy item is not a string: {}", host)
)) ))
} }
} }
}, },
None => return Err(WebDriverError::new( None => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not an array", value) format!("noProxy is not an array: {}", value)
)) ))
} }
@ -211,20 +218,20 @@ impl SpecNewSessionParameters {
/// Validate whether a named capability is JSON value is a string containing a host /// Validate whether a named capability is JSON value is a string containing a host
/// and possible port /// and possible port
fn validate_host(value: &Json) -> WebDriverResult<()> { fn validate_host(value: &Json, entry: &str) -> WebDriverResult<()> {
match value.as_string() { match value.as_string() {
Some(host) => { Some(host) => {
if host.contains("://") { if host.contains("://") {
return Err(WebDriverError::new( return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} contains a scheme", host))); format!("{} must not contain a scheme: {}", entry, host)));
} }
// Temporarily add a scheme so the host can be parsed as URL // Temporarily add a scheme so the host can be parsed as URL
let s = String::from(format!("http://{}", host)); let s = String::from(format!("http://{}", host));
let url = try!(Url::parse(s.as_str()).or(Err(WebDriverError::new( let url = Url::parse(s.as_str()).or(Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not a valid host", host))))); format!("{} is not a valid URL: {}", entry, host))))?;
if url.username() != "" || if url.username() != "" ||
url.password() != None || url.password() != None ||
@ -233,90 +240,122 @@ impl SpecNewSessionParameters {
url.fragment() != None { url.fragment() != None {
return Err(WebDriverError::new( return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not of the form host[:port]", host))); format!("{} is not of the form host[:port]: {}", entry, host)));
} }
}, },
None => return Err(WebDriverError::new( None => return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
format!("{} is not a string", value) format!("{} is not a string: {}", entry, value)
)) ))
} }
Ok(()) Ok(())
} }
fn validate_timeouts(value: &Json) -> WebDriverResult<()> { fn validate_timeouts(value: &Json) -> WebDriverResult<()> {
let obj = try_opt!(value.as_object(), let obj = try_opt!(
ErrorStatus::InvalidArgument, value.as_object(),
"timeouts capability is not an object"); ErrorStatus::InvalidArgument,
"timeouts capability is not an object"
);
for (key, value) in obj.iter() { for (key, value) in obj.iter() {
match &**key { match &**key {
x @ "script" | x @ "script" | x @ "pageLoad" | x @ "implicit" => {
x @ "pageLoad" | let timeout = try_opt!(
x @ "implicit" => { value.as_i64(),
let timeout = try_opt!(value.as_i64(), ErrorStatus::InvalidArgument,
ErrorStatus::InvalidArgument, format!("{} timeouts value is not an integer: {}", x, value)
format!("{} timeouts value is not an integer", x)); );
if timeout < 0 { if timeout < 0 {
return Err(WebDriverError::new(ErrorStatus::InvalidArgument, return Err(WebDriverError::new(
format!("{} timeouts value is negative", x))) ErrorStatus::InvalidArgument,
format!("{} timeouts value is negative: {}", x, timeout),
));
} }
}, }
x => return Err(WebDriverError::new(ErrorStatus::InvalidArgument,
format!("{} is not a valid timeouts capability", x))) x => {
return Err(WebDriverError::new(
ErrorStatus::InvalidArgument,
format!("Invalid timeouts capability entry: {}", x),
))
}
} }
} }
Ok(()) Ok(())
} }
fn validate_unhandled_prompt_behaviour(value: &Json) -> WebDriverResult<()> { fn validate_unhandled_prompt_behaviour(value: &Json) -> WebDriverResult<()> {
let behaviour = try_opt!(value.as_string(), let behaviour = try_opt!(
ErrorStatus::InvalidArgument, value.as_string(),
"unhandledPromptBehavior capability is not a string"); ErrorStatus::InvalidArgument,
format!("unhandledPromptBehavior is not a string: {}", value)
);
match behaviour { match behaviour {
"dismiss" | "dismiss" | "accept" => {}
"accept" => {}, x => {
x => return Err(WebDriverError::new(ErrorStatus::InvalidArgument, return Err(WebDriverError::new(
format!("{} is not a valid unhandledPromptBehavior value", x))) } ErrorStatus::InvalidArgument,
format!("Invalid unhandledPromptBehavior value: {}", x),
))
}
}
Ok(()) Ok(())
} }
} }
impl Parameters for SpecNewSessionParameters { impl Parameters for SpecNewSessionParameters {
fn from_json(body: &Json) -> WebDriverResult<SpecNewSessionParameters> { fn from_json(body: &Json) -> WebDriverResult<SpecNewSessionParameters> {
let data = try_opt!(body.as_object(), let data = try_opt!(
ErrorStatus::UnknownError, body.as_object(),
"Message body is not an object"); ErrorStatus::UnknownError,
format!("Malformed capabilities, message body is not an object: {}", body)
);
let capabilities = try_opt!( let capabilities = try_opt!(
try_opt!(data.get("capabilities"), try_opt!(
ErrorStatus::InvalidArgument, data.get("capabilities"),
"Missing 'capabilities' parameter").as_object(), ErrorStatus::InvalidArgument,
"Malformed capabilities, missing \"capabilities\" field"
).as_object(),
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"'capabilities' parameter is not an object"); "Malformed capabilities, \"capabilities\" field is not an object}"
);
let default_always_match = Json::Object(Capabilities::new()); let default_always_match = Json::Object(Capabilities::new());
let always_match = try_opt!(capabilities.get("alwaysMatch") let always_match = try_opt!(
.unwrap_or(&default_always_match) capabilities
.as_object(), .get("alwaysMatch")
ErrorStatus::InvalidArgument, .unwrap_or(&default_always_match)
"'alwaysMatch' parameter is not an object"); .as_object(),
ErrorStatus::InvalidArgument,
"Malformed capabilities, alwaysMatch field is not an object"
);
let default_first_matches = Json::Array(vec![]); let default_first_matches = Json::Array(vec![]);
let first_matches = try!( let first_matches = try_opt!(
try_opt!(capabilities.get("firstMatch") capabilities
.unwrap_or(&default_first_matches) .get("firstMatch")
.as_array(), .unwrap_or(&default_first_matches)
ErrorStatus::InvalidArgument, .as_array(),
"'firstMatch' parameter is not an array") ErrorStatus::InvalidArgument,
.iter() "Malformed capabilities, firstMatch field is not an array"
.map(|x| x.as_object() ).iter()
.map(|x| x.clone()) .map(|x| {
.ok_or(WebDriverError::new(ErrorStatus::InvalidArgument, x.as_object().map(|x| x.clone()).ok_or(WebDriverError::new(
"'firstMatch' entry is not an object"))) ErrorStatus::InvalidArgument,
.collect::<WebDriverResult<Vec<Capabilities>>>()); "Malformed capabilities, firstMatch entry is not an object",
))
})
.collect::<WebDriverResult<Vec<Capabilities>>>()?;
return Ok(SpecNewSessionParameters { return Ok(SpecNewSessionParameters {
alwaysMatch: always_match.clone(), alwaysMatch: always_match.clone(),
firstMatch: first_matches firstMatch: first_matches,
}); });
} }
} }
@ -347,13 +386,10 @@ impl CapabilitiesMatching for SpecNewSessionParameters {
let merged_capabilities = capabilities_list let merged_capabilities = capabilities_list
.iter() .iter()
.map(|first_match_entry| { .map(|first_match_entry| {
if first_match_entry.keys().any( if first_match_entry.keys().any(|k| self.alwaysMatch.contains_key(k)) {
|k| self.alwaysMatch.contains_key(k),
)
{
return Err(WebDriverError::new( return Err(WebDriverError::new(
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"'firstMatch' key shadowed a value in 'alwaysMatch'", "firstMatch key shadowed a value in alwaysMatch",
)); ));
} }
let mut merged = self.alwaysMatch.clone(); let mut merged = self.alwaysMatch.clone();
@ -482,32 +518,33 @@ impl CapabilitiesMatching for LegacyNewSessionParameters {
impl Parameters for LegacyNewSessionParameters { impl Parameters for LegacyNewSessionParameters {
fn from_json(body: &Json) -> WebDriverResult<LegacyNewSessionParameters> { fn from_json(body: &Json) -> WebDriverResult<LegacyNewSessionParameters> {
let data = try_opt!(body.as_object(), let data = try_opt!(
ErrorStatus::UnknownError, body.as_object(),
"Message body is not an object"); ErrorStatus::UnknownError,
format!("Malformed legacy capabilities, message body is not an object: {}")
);
let desired_capabilities = let desired = if let Some(capabilities) = data.get("desiredCapabilities") {
if let Some(capabilities) = data.get("desiredCapabilities") { try_opt!(
try_opt!(capabilities.as_object(), capabilities.as_object(),
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"'desiredCapabilities' parameter is not an object").clone() "Malformed legacy capabilities, desiredCapabilities field is not an object"
} else { ).clone()
BTreeMap::new() } else {
}; BTreeMap::new()
};
let required_capabilities = let required = if let Some(capabilities) = data.get("requiredCapabilities") {
if let Some(capabilities) = data.get("requiredCapabilities") { try_opt!(
try_opt!(capabilities.as_object(), capabilities.as_object(),
ErrorStatus::InvalidArgument, ErrorStatus::InvalidArgument,
"'requiredCapabilities' parameter is not an object").clone() "Malformed legacy capabilities, requiredCapabilities field is not an object"
} else { ).clone()
BTreeMap::new() } else {
}; BTreeMap::new()
};
Ok(LegacyNewSessionParameters { Ok(LegacyNewSessionParameters { desired, required })
desired: desired_capabilities,
required: required_capabilities
})
} }
} }
@ -523,7 +560,7 @@ impl ToJson for LegacyNewSessionParameters {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use rustc_serialize::json::Json; use rustc_serialize::json::Json;
use super::{WebDriverResult, SpecNewSessionParameters}; use super::{SpecNewSessionParameters, WebDriverResult};
fn validate_proxy(value: &str) -> WebDriverResult<()> { fn validate_proxy(value: &str) -> WebDriverResult<()> {
let data = Json::from_str(value).unwrap(); let data = Json::from_str(value).unwrap();

2
testing/webdriver/src/command.rs
Просмотреть файл

@ -353,7 +353,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
"Body was not a JSON Object")) "Body was not a JSON Object"))
} }
Err(json::ParserError::SyntaxError(_, line, col)) => { Err(json::ParserError::SyntaxError(_, line, col)) => {
let msg = format!("Failed to decode request as JSON: {}", body); let msg = format!("Failed to decode request as JSON: \"{}\"", body);
let stack = format!("Syntax error at :{}:{}", line, col); let stack = format!("Syntax error at :{}:{}", line, col);
Err(WebDriverError::new_with_stack(ErrorStatus::InvalidArgument, msg, stack)) Err(WebDriverError::new_with_stack(ErrorStatus::InvalidArgument, msg, stack))
} }