Commit f95a75f3 authored by Leonardo Schwarz's avatar Leonardo Schwarz

New error handling code.

parent 5972ce58
Pipeline #121447 failed with stage
in 1 minute and 12 seconds
......@@ -10,7 +10,7 @@ edition = "2018"
default = []
[dependencies]
error-chain = "0.11"
backtrace = "0.3"
isolang = "1"
regex = "1"
reqwest_mock = "0.5"
......
use super::*;
use xpath_reader::Reader;
use crate::{Error, ErrorKind};
/// Checks if there is an error in the document provided by the reader and
/// returns Ok if there
/// wasn't and Err parsing the MusicBrainz error if the API actually returned
/// an error.
pub fn check_response_error<'d>(reader: &'d Reader<'d>) -> Result<(), ClientError> {
pub fn check_response_error<'d>(reader: &'d Reader<'d>) -> Result<(), Error> {
let r: Result<Vec<String>, _> = reader.read("//error/text");
match r {
Ok(errs) => {
if errs.len() > 0 {
let text = errs.join("\n");
Err(ClientErrorKind::MusicbrainzServerError(text).into())
Err(Error::new(text, ErrorKind::ServerError))
} else {
Ok(())
}
......@@ -24,7 +24,6 @@ pub fn check_response_error<'d>(reader: &'d Reader<'d>) -> Result<(), ClientErro
#[cfg(test)]
mod tests {
use super::*;
use error_chain::ChainedError;
use xpath_reader::Reader;
const XML_ERR: &'static str = r#"<?xml version="1.0" encoding="UTF-8"?>
......@@ -43,9 +42,8 @@ mod tests {
let res = check_response_error(&reader);
let err = res.err().unwrap();
assert_eq!(err.description(), "MusicBrainz server error");
assert!(format!("{}", err).starts_with("MusicBrainz server error: Your requests are exceeding the allowable rate limit. Please see http://wiki.musicbrainz.org/XMLWebService for more information.\nFor usage, please see: http://musicbrainz.org/development/mmd"));
assert!(format!("{}", err).starts_with(
"[server error]: Your requests are exceeding the allowable rate limit. Please see http://wiki.musicbrainz.org/XMLWebService for more information.\nFor usage, please see: http://musicbrainz.org/development/mmd"));
}
#[test]
......
//! Contains the types and functions to communicate with the MusicBrainz API.
use crate::errors::{ClientError, ClientErrorKind};
use crate::errors::{Error, ErrorKind};
use crate::entities::{Mbid, Resource};
use reqwest_mock::Client as MockClient;
......@@ -129,7 +129,7 @@ impl Client {
}
/// Fetch the specified ressource from the server and parse it.
pub fn get_by_mbid<Res>(&mut self, mbid: &Mbid) -> Result<Res, ClientError>
pub fn get_by_mbid<Res>(&mut self, mbid: &Mbid) -> Result<Res, Error>
where
Res: Resource + FromXml,
{
......@@ -143,7 +143,7 @@ impl Client {
Ok(Res::from_xml(&reader)?)
}
pub(crate) fn get_body(&mut self, url: Url) -> Result<String, ClientError> {
pub(crate) fn get_body(&mut self, url: Url) -> Result<String, Error> {
self.wait_if_needed();
let mut attempts = 0;
......@@ -166,7 +166,7 @@ impl Client {
return Ok(response_body);
}
}
Err("MusicBrainz returned 503 (ServiceUnavailable) too many times.".into())
Err(Error::new("MusicBrainz returned 503 (ServiceUnavailable) too many times.", ErrorKind::Communication))
}
/// Returns a search builder to search for an area.
......
use std::time::Duration;
use xpath_reader::{Reader, FromXml, FromXmlOptional};
use xpath_reader::Reader;
/// Note that the requirement of the `var` (variant) token is rather ugly but
/// required,
......
use crate::errors::ParseError;
use crate::errors::Error;
use isolang::Language as IsoLang;
use std::fmt;
use xpath_reader::{FromXml, FromXmlOptional, Error, Reader};
use xpath_reader::{FromXml, FromXmlOptional, Reader};
/// Represents verbal languages.
#[derive(Clone, Eq, PartialEq)]
......@@ -11,20 +11,20 @@ pub struct Language {
impl Language {
/// Construct a new instance from an ISO 639-1 language code.
pub fn from_639_1(code: &str) -> Result<Language, ParseError> {
pub fn from_639_1(code: &str) -> Result<Language, Error> {
Ok(Language {
inner: IsoLang::from_639_1(code)
.ok_or_else(|| ParseError::from(format!("Invalid ISO 639-1 code: {}", code)))?,
.ok_or_else(|| Error::parse_error(format!("Invalid ISO 639-1 code: {}", code)))?,
})
}
/// Construct a new instance from an ISO 639-3 language code.
///
/// These are used by MusicBrainz internally.
pub fn from_639_3(code: &str) -> Result<Language, ParseError> {
pub fn from_639_3(code: &str) -> Result<Language, Error> {
Ok(Language {
inner: IsoLang::from_639_3(code)
.ok_or_else(|| ParseError::from(format!("Invalid ISO 639-3 code: {}", code)))?,
.ok_or_else(|| Error::parse_error(format!("Invalid ISO 639-3 code: {}", code)))?,
})
}
......@@ -46,7 +46,7 @@ impl fmt::Debug for Language {
}
impl FromXmlOptional for Language {
fn from_xml_optional<'d>(reader: &'d Reader<'d>) -> Result<Option<Self>, Error> {
fn from_xml_optional<'d>(reader: &'d Reader<'d>) -> Result<Option<Self>, xpath_reader::Error> {
let s = Option::<String>::from_xml(&reader)?;
if let Some(s) = s {
......
use std::fmt::{self, Debug, Display, Formatter};
use std::str::FromStr;
use uuid::{self, Uuid};
use xpath_reader::{FromXml, Error, Reader};
use xpath_reader::{FromXml, Reader};
/// Identifier for entities in the MusicBrainz database.
#[derive(Clone, PartialEq, Eq)]
......
......@@ -4,19 +4,18 @@
// TODO: Better documentation in this file.
use std::time::Duration;
use xpath_reader::{FromXml, FromXmlOptional, Error, Reader};
use xpath_reader::{FromXml, FromXmlOptional, Reader};
use crate::entities::Mbid;
use crate::entities::date::PartialDate;
use crate::entities::release::ReleaseStatus;
use crate::client::Client;
use crate::errors::ClientError;
use crate::Error;
pub trait FetchFull {
type Full;
fn fetch_full(&self, client: &mut Client) -> Result<Self::Full, ClientError>;
fn fetch_full(&self, client: &mut Client) -> Result<Self::Full, Error>;
}
#[derive(Clone, Debug, Eq, PartialEq)]
......@@ -28,7 +27,7 @@ pub struct AreaRef {
}
impl FromXmlOptional for AreaRef {
fn from_xml_optional<'d>(reader: &'d Reader<'d>) -> Result<Option<Self>, Error> {
fn from_xml_optional<'d>(reader: &'d Reader<'d>) -> Result<Option<Self>, xpath_reader::Error> {
// TODO: is this correct
if reader.anchor_nodeset().size() < 1 {
Ok(None)
......@@ -51,7 +50,7 @@ pub struct ArtistRef {
}
impl FromXml for ArtistRef {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, Error> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, xpath_reader::Error> {
Ok(ArtistRef {
mbid: reader.read(".//@id")?,
name: reader.read(".//mb:name/text()")?,
......@@ -69,7 +68,7 @@ pub struct LabelRef {
}
impl FromXml for LabelRef {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, Error> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, xpath_reader::Error> {
Ok(LabelRef {
mbid: reader.read(".//@id")?,
name: reader.read(".//mb:name/text()")?,
......@@ -87,7 +86,7 @@ pub struct RecordingRef {
}
impl FromXml for RecordingRef {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, Error> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, xpath_reader::Error> {
Ok(RecordingRef {
mbid: reader.read(".//@id")?,
title: reader.read(".//mb:title/text()")?,
......@@ -106,7 +105,7 @@ pub struct ReleaseRef {
}
impl FromXml for ReleaseRef {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, Error> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, xpath_reader::Error> {
Ok(ReleaseRef {
mbid: reader.read(".//@id")?,
title: reader.read(".//mb:title/text()")?,
......@@ -126,7 +125,7 @@ macro_rules! ref_fetch_full
impl FetchFull for $ref {
type Full = $full;
fn fetch_full(&self, client: &mut Client) -> Result<Self::Full, ClientError>
fn fetch_full(&self, client: &mut Client) -> Result<Self::Full, Error>
{
client.get_by_mbid(&self.mbid)
}
......
// TODO: Actually the docs generated by error chain are rather misleading since
// they are exactly the same for both ClientError and ParseError.
//
// One work around would be to put each error chain in its own module and
// document that module, then reexport the types.
error_chain! {
types {
ParseError, ParseErrorKind, ChainParseErr;
}
use backtrace::Backtrace;
use std::fmt;
links {
}
#[derive(Clone, Debug)]
pub struct Error {
// TODO: Make it possible to disable backtraces for performance reasons?
backtrace: Backtrace,
message: String,
kind: ErrorKind,
}
#[derive(Clone, Debug, Eq, PartialEq, Copy)]
pub(crate) enum ErrorKind {
/// Something went wrong while parsing a response.
ParseResponse,
/// Internal error of other kinds.
Internal,
/// Communication failed.
Communication,
// Automatic conversions between this error chain and errors not defined using error
// chain.
foreign_links {
UuidParseError(::uuid::parser::ParseError);
ParseIntError(::std::num::ParseIntError);
ParseDateError(super::entities::ParseDateError);
/// The server returned an error message.
ServerError,
}
impl ErrorKind {
/// True if an error of this kind constitutes a bug that should ideally be reported to upstream.
pub fn is_bug(&self) -> bool {
match self {
ErrorKind::ParseResponse | ErrorKind::Internal => true,
ErrorKind::Communication | ErrorKind::ServerError => false,
}
}
}
// Custom error kinds.
errors {
InvalidData(msg: String) {
description("invalid data")
display("invalid data: {}", msg)
impl Error {
pub(crate) fn new<S: Into<String>>(msg: S, kind: ErrorKind) -> Error {
Error {
message: msg.into(),
kind,
backtrace: Backtrace::new()
}
/// Somewhere in our code something went wrong, that really shouldn't have.
/// These are always considered a bug that should reported as an issue.
InternalError(msg: String) {
description("internal error")
display("internal error: {}\nYou should probably report this bug.", msg)
}
pub(crate) fn parse_error<S: Into<String>>(msg: S) -> Error {
Error {
message: msg.into(),
kind: ErrorKind::ParseResponse,
backtrace: Backtrace::new(),
}
}
}
error_chain! {
types {
ClientError, ClientErrorKind, ChainClientErr;
impl std::error::Error for Error {
}
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
match self.kind {
ErrorKind::ParseResponse => {
writeln!(f, "[parse response error]: {}", self.message)?;
}
ErrorKind::Internal => {
writeln!(f, "[internal error]: {}", self.message)?;
}
ErrorKind::Communication => {
writeln!(f, "[communication error]: {}", self.message)?;
}
ErrorKind::ServerError => {
writeln!(f, "[server error]: {}", self.message)?;
}
}
if self.kind.is_bug() {
writeln!(f, "This might be a bug that should be reported upstream.")?;
}
writeln!(f, "Backtrace: {:?}", self.backtrace)?;
Ok(())
}
}
links {
ParseError(ParseError, ParseErrorKind);
ReqwestMockError(::reqwest_mock::Error, ::reqwest_mock::error::ErrorKind);
impl From<xpath_reader::Error> for Error {
fn from(e: xpath_reader::Error) -> Self {
Error {
message: format!("xpath_reader error: {}", e),
kind: ErrorKind::ParseResponse,
backtrace: Backtrace::new(),
}
}
}
foreign_links {
IoError(::std::io::Error);
Url(::reqwest_mock::UrlError);
Error(::xpath_reader::Error);
impl From<reqwest_mock::Error> for Error {
fn from(e: reqwest_mock::Error) -> Self {
Error {
message: format!("reqwest_mock parse error: {}", e),
kind: ErrorKind::Internal,
backtrace: Backtrace::new(),
}
}
}
errors {
/// This is most likely a rate limit, but since the errors aren't coded and there are
/// also other issues like the server being busy we don't distinguish it except for the
/// provided message.
MusicbrainzServerError(msg: String) {
description("MusicBrainz server error")
display("MusicBrainz server error: {}", msg)
impl From<reqwest_mock::UrlError> for Error {
fn from(e: reqwest_mock::UrlError) -> Self {
Error {
message: format!("reqwest_mock url error: {}", e),
kind: ErrorKind::Internal,
backtrace: Backtrace::new()
}
}
}
......@@ -3,8 +3,6 @@
// TODO: Remove before stable release.
#![allow(dead_code)]
#[macro_use]
extern crate error_chain;
extern crate isolang;
extern crate regex;
extern crate reqwest_mock;
......
......@@ -15,12 +15,12 @@
use crate::entities as full_entities;
use crate::entities::Resource;
use crate::errors::ClientError;
use crate::errors::Error;
use crate::client::Client;
use reqwest_mock::Url;
use url::percent_encoding::{DEFAULT_ENCODE_SET, utf8_percent_encode};
use xpath_reader::{FromXml, Error, Reader};
use xpath_reader::{FromXml, Reader};
pub mod fields;
use self::fields::{AreaSearchField, ArtistSearchField, ReleaseGroupSearchField, ReleaseSearchField};
......@@ -28,7 +28,7 @@ use self::fields::{AreaSearchField, ArtistSearchField, ReleaseGroupSearchField,
pub mod search_entities;
use self::search_entities::SearchEntity;
pub type SearchResult<Entity> = Result<Vec<SearchEntry<Entity>>, ClientError>;
pub type SearchResult<Entity> = Result<Vec<SearchEntry<Entity>>, Error>;
pub mod query;
......@@ -89,7 +89,7 @@ macro_rules! define_search_builder {
}
/// Builds the full url to be used to perform the search request.
fn build_url(&self) -> Result<Url, ClientError> {
fn build_url(&self) -> Result<Url, Error> {
let mut query_parts: Vec<String> = Vec::new();
for &(p_name, ref p_value) in self.params.iter() {
// TODO (FIXME): Does this also encode ":" ?
......@@ -131,7 +131,7 @@ macro_rules! define_search_builder {
}
impl FromXml for SearchEntry<$entity> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, Error> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, xpath_reader::Error> {
Ok(Self {
entity: reader.read(format!(".//mb:{}", $list_tag).as_str())?,
score: reader.read(format!(".//mb:{}/*/@ext:score", $list_tag).as_str())?,
......
......@@ -4,10 +4,10 @@
//! They are only contained in search results and provide a means to retrive
//! the full entitity a further API request.
use super::{Client, ClientError, full_entities};
use super::{Client, full_entities};
use crate::Error;
use self::full_entities::refs::*;
use self::full_entities::{Mbid, Resource};
use xpath_reader::Error;
use xpath_reader::reader::{FromXml, Reader};
pub trait SearchEntity {
......@@ -15,7 +15,7 @@ pub trait SearchEntity {
type FullEntity: Resource + FromXml;
/// Fetch the full entity from the API.
fn fetch_full(&self, client: &mut Client) -> Result<Self::FullEntity, ClientError>;
fn fetch_full(&self, client: &mut Client) -> Result<Self::FullEntity, Error>;
}
// It's the same entity.
......@@ -24,7 +24,7 @@ pub use self::full_entities::Area;
impl SearchEntity for Area {
type FullEntity = Area;
fn fetch_full(&self, _: &mut Client) -> Result<Self::FullEntity, ClientError> {
fn fetch_full(&self, _: &mut Client) -> Result<Self::FullEntity, Error> {
Ok(self.to_owned())
}
}
......@@ -34,7 +34,7 @@ pub use self::full_entities::Artist;
impl SearchEntity for Artist {
type FullEntity = Artist;
fn fetch_full(&self, _: &mut Client) -> Result<Self::FullEntity, ClientError> {
fn fetch_full(&self, _: &mut Client) -> Result<Self::FullEntity, Error> {
Ok(self.to_owned())
}
}
......@@ -52,7 +52,7 @@ pub struct Release {
impl SearchEntity for Release {
type FullEntity = full_entities::Release;
fn fetch_full(&self, client: &mut Client) -> Result<Self::FullEntity, ClientError> {
fn fetch_full(&self, client: &mut Client) -> Result<Self::FullEntity, Error> {
client.get_by_mbid(&self.mbid)
}
}
......@@ -67,13 +67,13 @@ pub struct ReleaseGroup {
impl SearchEntity for ReleaseGroup {
type FullEntity = full_entities::ReleaseGroup;
fn fetch_full(&self, client: &mut Client) -> Result<Self::FullEntity, ClientError> {
fn fetch_full(&self, client: &mut Client) -> Result<Self::FullEntity, Error> {
client.get_by_mbid(&self.mbid)
}
}
impl FromXml for ReleaseGroup {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, Error> {
fn from_xml<'d>(reader: &'d Reader<'d>) -> Result<Self, xpath_reader::Error> {
Ok(ReleaseGroup {
mbid: reader.read(".//@id")?,
title: reader.read(".//mb:title")?,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment