Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving the error handling out of network loader... #8851

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Next

Propagating the load errors from network loader

  • Loading branch information
wafflespeanut committed Feb 20, 2016
commit 9887c33d09e627c4b9f006dbd1329c899e1d782f
@@ -173,15 +173,21 @@ impl FontCache {
ROUTER.add_route(data_receiver.to_opaque(), box move |message| {
let response: ResponseAction = message.to().unwrap();
match response {
ResponseAction::HeadersAvailable(metadata) => {
let is_response_valid =
metadata.content_type.as_ref().map_or(false, |content_type| {
let mime = &content_type.0;
is_supported_font_type(&mime.0, &mime.1)
});
info!("{} font with MIME type {:?}",
ResponseAction::HeadersAvailable(meta_result) => {
let is_response_valid = match meta_result {
Ok(ref metadata) => {
metadata.content_type.as_ref().map_or(false, |content_type| {
let mime = &content_type.0;
is_supported_font_type(&mime.0, &mime.1)
})
}
Err(_) => false,
};

info!("{} font with MIME type {}",
if is_response_valid { "Loading" } else { "Ignoring" },
metadata.content_type);
meta_result.map(|ref meta| format!("{:?}", meta.content_type))
.unwrap_or(format!("<Network Error>")));
*response_valid.lock().unwrap() = is_response_valid;
}
ResponseAction::DataAvailable(new_bytes) => {
@@ -9,7 +9,7 @@ use hyper::mime::{Mime, SubLevel, TopLevel};
use mime_classifier::MIMEClassifier;
use net_traits::ProgressMsg::Done;
use net_traits::response::HttpsState;
use net_traits::{LoadConsumer, LoadData, Metadata};
use net_traits::{LoadConsumer, LoadData, Metadata, NetworkError};
use resource_thread::{CancellationListener, send_error, start_sending_sniffed_opt};
use std::sync::Arc;
use url::Url;
@@ -49,7 +49,7 @@ pub fn factory(mut load_data: LoadData,
load_data.url = Url::from_file_path(&*path).unwrap();
}
_ => {
send_error(load_data.url, "Unknown about: URL.".to_owned(), start_chan);
send_error(load_data.url, NetworkError::Internal("Unknown about: URL.".to_owned()), start_chan);
return
}
};
@@ -5,7 +5,7 @@
use hyper::mime::{Mime, TopLevel, SubLevel, Attr, Value};
use mime_classifier::MIMEClassifier;
use net_traits::ProgressMsg::{Done, Payload};
use net_traits::{LoadConsumer, LoadData, Metadata};
use net_traits::{LoadConsumer, LoadData, Metadata, NetworkError};
use resource_thread::{CancellationListener, send_error, start_sending_sniffed_opt};
use rustc_serialize::base64::FromBase64;
use std::sync::Arc;
@@ -44,7 +44,7 @@ pub fn load(load_data: LoadData,
}
let parts: Vec<&str> = scheme_data.splitn(2, ',').collect();
if parts.len() != 2 {
send_error(url, "invalid data uri".to_owned(), start_chan);
send_error(url, NetworkError::Internal("Invalid Data URI".to_owned()), start_chan);
return;
}

@@ -79,7 +79,7 @@ pub fn load(load_data: LoadData,
// but Acid 3 apparently depends on spaces being ignored.
let bytes = bytes.into_iter().filter(|&b| b != ' ' as u8).collect::<Vec<u8>>();
match bytes.from_base64() {
Err(..) => return send_error(url, "non-base64 data uri".to_owned(), start_chan),
Err(..) => return send_error(url, NetworkError::Internal("Non-Base64 Data URI".to_owned()), start_chan),
Ok(data) => data,
}
} else {
@@ -6,7 +6,7 @@ use about_loader;
use mime_classifier::MIMEClassifier;
use mime_guess::guess_mime_type;
use net_traits::ProgressMsg::{Done, Payload};
use net_traits::{LoadConsumer, LoadData, Metadata};
use net_traits::{LoadConsumer, LoadData, Metadata, NetworkError};
use resource_thread::{CancellationListener, ProgressSender};
use resource_thread::{send_error, start_sending_sniffed_opt};
use std::borrow::ToOwned;
@@ -46,7 +46,7 @@ fn read_all(reader: &mut File, progress_chan: &ProgressSender, cancel_listener:
-> Result<LoadResult, String> {
loop {
if cancel_listener.is_cancelled() {
let _ = progress_chan.send(Done(Err("load cancelled".to_owned())));
let _ = progress_chan.send(Done(Err(NetworkError::Internal("load cancelled".to_owned()))));
return Ok(LoadResult::Cancelled);
}

@@ -80,7 +80,9 @@ pub fn factory(load_data: LoadData,
if cancel_listener.is_cancelled() {
if let Ok(progress_chan) = get_progress_chan(load_data, file_path,
senders, classifier, &[]) {
let _ = progress_chan.send(Done(Err("load cancelled".to_owned())));
let _ = progress_chan.send(
Done(Err(NetworkError::Internal("load cancelled".to_owned())))
);
}
return;
}
@@ -104,7 +106,7 @@ pub fn factory(load_data: LoadData,
}
}
Err(e) => {
send_error(load_data.url, e, senders);
send_error(load_data.url, NetworkError::Internal(e), senders);
}
};
}
@@ -119,7 +121,7 @@ pub fn factory(load_data: LoadData,
}
}
Err(_) => {
send_error(load_data.url, "Could not parse path".to_owned(), senders);
send_error(load_data.url, NetworkError::Internal("Could not parse path".to_owned()), senders);
}
}
});
@@ -8,7 +8,6 @@ use cookie;
use cookie_storage::CookieStorage;
use devtools_traits::{ChromeToDevtoolsControlMsg, DevtoolsControlMsg, HttpRequest as DevtoolsHttpRequest};
use devtools_traits::{HttpResponse as DevtoolsHttpResponse, NetworkEvent};
use file_loader;
use flate2::read::{DeflateDecoder, GzDecoder};
use hsts::{HSTSEntry, HSTSList, secure_url};
use hyper::Error as HttpError;
@@ -27,7 +26,8 @@ use msg::constellation_msg::{PipelineId};
use net_traits::ProgressMsg::{Done, Payload};
use net_traits::hosts::replace_hosts;
use net_traits::response::HttpsState;
use net_traits::{CookieSource, IncludeSubdomains, LoadConsumer, LoadContext, LoadData, Metadata};
use net_traits::{CookieSource, IncludeSubdomains, LoadConsumer, LoadContext, LoadData};
use net_traits::{Metadata, NetworkError};
use openssl::ssl::error::{SslError, OpensslError};
use openssl::ssl::{SSL_OP_NO_SSLV2, SSL_OP_NO_SSLV3, SSL_VERIFY_PEER, SslContext, SslMethod};
use resource_thread::{CancellationListener, send_error, start_sending_sniffed_opt};
@@ -141,32 +141,15 @@ fn load_for_consumer(load_data: LoadData,
cookie_jar, devtools_chan,
&factory, user_agent,
&cancel_listener) {
Err(LoadError::UnsupportedScheme(url)) => {
let s = format!("{} request, but we don't support that scheme", &*url.scheme);
send_error(url, s, start_chan)
}
Err(LoadError::Connection(url, e)) => {
send_error(url, e, start_chan)
}
Err(LoadError::MaxRedirects(url)) => {
send_error(url, "too many redirects".to_owned(), start_chan)
}
Err(LoadError::Cors(url, msg)) |
Err(LoadError::Cancelled(url, msg)) |
Err(LoadError::InvalidRedirect(url, msg)) |
Err(LoadError::Decoding(url, msg)) => {
send_error(url, msg, start_chan)
}
Err(LoadError::Ssl(url, msg)) => {
info!("ssl validation error {}, '{}'", url.serialize(), msg);

let mut image = resources_dir_path();
image.push("badcert.html");
let load_data = LoadData::new(context, Url::from_file_path(&*image).unwrap(), None);

file_loader::factory(load_data, start_chan, classifier, cancel_listener)
Err(error) => {
match error.error {
LoadErrorType::ConnectionAborted => unreachable!(),
LoadErrorType::Ssl => send_error(error.url.clone(),
NetworkError::SslValidation(error.url),
start_chan),
_ => send_error(error.url, NetworkError::Internal(error.reason), start_chan)
}
}
Err(LoadError::ConnectionAborted(_)) => unreachable!(),
Ok(mut load_response) => {
let metadata = load_response.metadata.clone();
send_data(context, &mut load_response, start_chan, metadata, classifier, &cancel_listener)
@@ -250,20 +233,15 @@ impl HttpRequestFactory for NetworkHttpRequestFactory {
let error: &(Error + Send + 'static) = &**error;
if let Some(&SslError::OpenSslErrors(ref errors)) = error.downcast_ref::<SslError>() {
if errors.iter().any(is_cert_verify_error) {
return Err(
LoadError::Ssl(url, format!("ssl error: {:?} {:?}",
error.description(),
error.cause())));
let msg = format!("ssl error: {:?} {:?}", error.description(), error.cause());
return Err(LoadError::new(url, LoadErrorType::Ssl, msg));
}
}
}

let request = match connection {
Ok(req) => req,

Err(e) => {
return Err(LoadError::Connection(url, e.description().to_owned()))
}
Err(e) => return Err(LoadError::new(url, LoadErrorType::Connection, e.description().to_owned())),
};

Ok(WrappedHttpRequest { request: request })
@@ -292,38 +270,57 @@ impl HttpRequest for WrappedHttpRequest {
let url = self.request.url.clone();
let mut request_writer = match self.request.start() {
Ok(streaming) => streaming,
Err(e) => return Err(LoadError::Connection(url, e.description().to_owned()))
Err(e) => return Err(LoadError::new(url, LoadErrorType::Connection, e.description().to_owned())),
};

if let Some(ref data) = *body {
if let Err(e) = request_writer.write_all(&data) {
return Err(LoadError::Connection(url, e.description().to_owned()))
return Err(LoadError::new(url, LoadErrorType::Connection, e.description().to_owned()))
}
}

let response = match request_writer.send() {
Ok(w) => w,
Err(HttpError::Io(ref io_error)) if io_error.kind() == io::ErrorKind::ConnectionAborted => {
return Err(LoadError::ConnectionAborted(io_error.description().to_owned()));
return Err(LoadError::new(url, LoadErrorType::ConnectionAborted,
io_error.description().to_owned()));
},
Err(e) => return Err(LoadError::Connection(url, e.description().to_owned()))
Err(e) => return Err(LoadError::new(url, LoadErrorType::Connection,
e.description().to_owned())),
};

Ok(WrappedHttpResponse { response: response })
}
}

#[derive(Debug)]
pub enum LoadError {
UnsupportedScheme(Url),
Connection(Url, String),
Cors(Url, String),
Ssl(Url, String),
InvalidRedirect(Url, String),
Decoding(Url, String),
MaxRedirects(Url),
ConnectionAborted(String),
Cancelled(Url, String),
pub struct LoadError {
url: Url,
error: LoadErrorType,
reason: String,
}

impl LoadError {
fn new(url: Url, error: LoadErrorType, reason: String) -> LoadError {
LoadError {
url: url,
error: error,
reason: reason,
}
}
}

#[derive(Debug)]
pub enum LoadErrorType {
Cancelled,
Connection,
ConnectionAborted,
Cors,
Decoding,
InvalidRedirect,
MaxRedirects,
Ssl,
UnsupportedScheme,
}

fn set_default_accept_encoding(headers: &mut Headers) {
@@ -442,12 +439,8 @@ impl<R: HttpResponse> StreamedResponse<R> {
Some(Encoding::Gzip) => {
let result = GzDecoder::new(response);
match result {
Ok(response_decoding) => {
Ok(StreamedResponse::new(m, Decoder::Gzip(response_decoding)))
}
Err(err) => {
Err(LoadError::Decoding(m.final_url, err.to_string()))
}
Ok(response_decoding) => Ok(StreamedResponse::new(m, Decoder::Gzip(response_decoding))),
Err(err) => Err(LoadError::new(m.final_url, LoadErrorType::Decoding, err.to_string())),
}
}
Some(Encoding::Deflate) => {
@@ -576,7 +569,7 @@ pub fn obtain_response<A>(request_factory: &HttpRequestFactory<R=A>,
*req.headers_mut() = request_headers.clone();

if cancel_listener.is_cancelled() {
return Err(LoadError::Cancelled(url.clone(), "load cancelled".to_owned()));
return Err(LoadError::new(url.clone(), LoadErrorType::Cancelled, "load cancelled".to_owned()));
}

if log_enabled!(log::LogLevel::Info) {
@@ -619,11 +612,14 @@ pub fn obtain_response<A>(request_factory: &HttpRequestFactory<R=A>,

response = match maybe_response {
Ok(r) => r,
Err(LoadError::ConnectionAborted(reason)) => {
debug!("connection aborted ({:?}), possibly stale, trying new connection", reason);
continue;
}
Err(e) => return Err(e),
Err(e) => {
if let LoadErrorType::ConnectionAborted = e.error {
debug!("connection aborted ({:?}), possibly stale, trying new connection", e.reason);
continue;
} else {
return Err(e)
}
},
};

// if no ConnectionAborted, break the loop
@@ -655,7 +651,7 @@ pub fn load<A>(load_data: LoadData,
let mut method = load_data.method.clone();

if cancel_listener.is_cancelled() {
return Err(LoadError::Cancelled(url, "load cancelled".to_owned()));
return Err(LoadError::new(url, LoadErrorType::Cancelled, "load cancelled".to_owned()));
}

// If the URL is a view-source scheme then the scheme data contains the
@@ -678,15 +674,16 @@ pub fn load<A>(load_data: LoadData,
}

if iters > max_redirects {
return Err(LoadError::MaxRedirects(url));
return Err(LoadError::new(url, LoadErrorType::MaxRedirects, "too many redirects".to_owned()));
}

if &*url.scheme != "http" && &*url.scheme != "https" {
return Err(LoadError::UnsupportedScheme(url));
let s = format!("{} request, but we don't support that scheme", &*url.scheme);
return Err(LoadError::new(url, LoadErrorType::UnsupportedScheme, s));
}

if cancel_listener.is_cancelled() {
return Err(LoadError::Cancelled(url, "load cancelled".to_owned()));
return Err(LoadError::new(url, LoadErrorType::Cancelled, "load cancelled".to_owned()));
}

info!("requesting {}", url.serialize());
@@ -719,10 +716,9 @@ pub fn load<A>(load_data: LoadData,
// CORS (https://fetch.spec.whatwg.org/#http-fetch, status section, point 9, 10)
if let Some(ref c) = load_data.cors {
if c.preflight {
return Err(
LoadError::Cors(
url,
"Preflight fetch inconsistent with main fetch".to_owned()));
return Err(LoadError::new(url,
LoadErrorType::Cors,
"Preflight fetch inconsistent with main fetch".to_owned()));
} else {
// XXXManishearth There are some CORS-related steps here,
// but they don't seem necessary until credentials are implemented
@@ -731,9 +727,7 @@ pub fn load<A>(load_data: LoadData,

let new_doc_url = match doc_url.join(&new_url) {
Ok(u) => u,
Err(e) => {
return Err(LoadError::InvalidRedirect(doc_url, e.to_string()));
}
Err(e) => return Err(LoadError::new(doc_url, LoadErrorType::InvalidRedirect, e.to_string())),
};

info!("redirecting to {}", new_doc_url);
@@ -749,7 +743,7 @@ pub fn load<A>(load_data: LoadData,
}

if redirected_to.contains(&url) {
return Err(LoadError::InvalidRedirect(doc_url, "redirect loop".to_owned()));
return Err(LoadError::new(doc_url, LoadErrorType::InvalidRedirect, "redirect loop".to_owned()));
}

redirected_to.insert(doc_url.clone());
@@ -809,7 +803,7 @@ fn send_data<R: Read>(context: LoadContext,

loop {
if cancel_listener.is_cancelled() {
let _ = progress_chan.send(Done(Err("load cancelled".to_owned())));
let _ = progress_chan.send(Done(Err(NetworkError::Internal("load cancelled".to_owned()))));
return;
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.