Permalink
Browse files

refactor(bounces): pull all bounce/complaint code into one module

This change does two things:

1. The old `BounceRecord`, `BounceType` and `BounceSubtype` types are
   removed from the `auth_db` module into `bounces`, so that they can
   more easily be re-used when writing bounce/complaint data to Redis.

2. Those types are renamed to `DeliveryProblem`, `ProblemType` and
   `ProblemSubtype` respectively, and the `bounces` module is renamed to
   `delivery_problems`. That may seem weird at first, but I have good
   reason for it and am open to alternative names if anyone can suggest
   something better.

   Historically we've always treated complaints as a type of bounce
   event, but that's inaccurate because bounces are never preceded by a
   delivery event whereas complaints are always preceded by one. This
   has caught me out in the past when analysing metrics and expecting a
   blanket sum of `deliveries + bounces = sends` to be true. It isn't.
   It's only true if you filter complaints from bounces before
   calculating the sum.

   Because of that, I wanted some different nomenclature that didn't use
   "bounces" as an umbrella term for "bounces and complaints". It won't
   affect the names we use in the metrics but I think it's important for
   language we use in the codebase to be as precise as possible. Things
   like "delivery error" or "delivery failure" also seemed inaccurate
   for the same reason; a complaint implies that the actual delivery
   succeeded. Hence "delivery problems", which seems generic enough to
   legitimately include both event types.
  • Loading branch information...
philbooth committed Oct 5, 2018
1 parent 636e44f commit 9f133af9f1fd5ab0e4285a07a12a36207b962e41
View
8 API.md
@@ -21,7 +21,7 @@
- `code: 500, errno: 103`: Invalid Provider
- `code: 500, errno: 104`: Provider Error
- `code: 500, errno: 105`: Email Parsing Error
- `code: 429, errno: 106`: Bounce Complaint Error
- `code: 429, errno: 106`: Complaint Error
- `code: 429, errno: 107`: Bounce Soft Error
- `code: 429, errno: 108`: Bounce Hard Error
- `code: 500, errno: 109`: AuthDb Error
@@ -45,9 +45,9 @@ The following errors include additional response properties:
- `errno: 103`
- `errno: 104` name
- `errno: 105`
- `errno: 106` address, bouncedAt, bouce
- `errno: 107` address, bouncedAt, bouce
- `errno: 108` address, bouncedAt, bouce
- `errno: 106` address, bouncedAt, bounce
- `errno: 107` address, bouncedAt, bounce
- `errno: 108` address, bouncedAt, bounce
- `errno: 109`
- `errno: 110`
- `errno: 111`
View
@@ -16,7 +16,7 @@ use rocket::{
use rocket_contrib::{Json, JsonValue};
use serde_json::{map::Map, ser::to_string, Value};
use auth_db::BounceRecord;
use delivery_problems::DeliveryProblem;
use email_address::EmailAddress;
use logging::MozlogLogger;
@@ -133,22 +133,22 @@ pub enum AppErrorKind {
/// An error for when a bounce violation happens.
#[fail(display = "Email account sent complaint")]
BounceComplaintError {
ComplaintError {
address: EmailAddress,
bounced_at: u64,
bounce: BounceRecord,
bounce: DeliveryProblem,
},
#[fail(display = "Email account soft bounced")]
BounceSoftError {
address: EmailAddress,
bounced_at: u64,
bounce: BounceRecord,
bounce: DeliveryProblem,
},
#[fail(display = "Email account hard bounced")]
BounceHardError {
address: EmailAddress,
bounced_at: u64,
bounce: BounceRecord,
bounce: DeliveryProblem,
},
/// An error occurred inside an auth db method.
@@ -215,7 +215,7 @@ impl AppErrorKind {
AppErrorKind::MethodNotAllowed => Status::MethodNotAllowed,
AppErrorKind::UnprocessableEntity => Status::UnprocessableEntity,
AppErrorKind::TooManyRequests => Status::TooManyRequests,
AppErrorKind::BounceComplaintError { .. }
AppErrorKind::ComplaintError { .. }
| AppErrorKind::BounceSoftError { .. }
| AppErrorKind::BounceHardError { .. } => Status::TooManyRequests,
AppErrorKind::BadRequest | AppErrorKind::InvalidEmailParams => Status::BadRequest,
@@ -235,7 +235,7 @@ impl AppErrorKind {
AppErrorKind::ProviderError { .. } => Some(104),
AppErrorKind::EmailParsingError(_) => Some(105),
AppErrorKind::BounceComplaintError { .. } => Some(106),
AppErrorKind::ComplaintError { .. } => Some(106),
AppErrorKind::BounceSoftError { .. } => Some(107),
AppErrorKind::BounceHardError { .. } => Some(108),
@@ -274,7 +274,7 @@ impl AppErrorKind {
fields.insert(String::from("name"), Value::String(format!("{}", name)));
}
AppErrorKind::BounceComplaintError {
AppErrorKind::ComplaintError {
ref address,
ref bounced_at,
ref bounce,
View
@@ -11,205 +11,32 @@
//! this module MUST NOT be used directly.
//! Instead,
//! all access should be via
//! [`bounces::Bounces`][bounces].
//! [`delivery_problems::DeliveryProblems`][delpro].
//!
//! [authdb]: https://github.com/mozilla/fxa-auth-db-mysql/
//! [bounces]: ../bounces/struct.Bounces.html
//! [delpro]: ../delivery_problems/struct.DeliveryProblems.html
use std::fmt::Debug;
use hex;
use reqwest::{Client as RequestClient, Error as RequestError, StatusCode, Url, UrlError};
use serde::{
de::{Deserialize, Deserializer, Error as DeserializeError, Unexpected},
ser::{Serialize, Serializer},
};
use app_errors::{AppError, AppErrorKind, AppResult};
use delivery_problems::{DeliveryProblem, ProblemSubtype, ProblemType};
use email_address::EmailAddress;
use settings::Settings;
#[cfg(test)]
mod test;
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub enum BounceType {
Hard,
Soft,
Complaint,
}
impl<'d> Deserialize<'d> for BounceType {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'d>,
{
let value: u8 = Deserialize::deserialize(deserializer)?;
match value {
// The auth db falls back to zero when it receives a value it doesn't recognise
0 => {
println!("Mapped default auth db bounce type to BounceType::Soft");
Ok(BounceType::Soft)
}
1 => Ok(BounceType::Hard),
2 => Ok(BounceType::Soft),
3 => Ok(BounceType::Complaint),
_ => Err(D::Error::invalid_value(
Unexpected::Unsigned(u64::from(value)),
&"bounce type",
)),
}
}
}
impl Serialize for BounceType {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let value = match self {
BounceType::Hard => 1,
BounceType::Soft => 2,
BounceType::Complaint => 3,
};
serializer.serialize_u8(value)
}
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum BounceSubtype {
// Set by the auth db if an input string is not recognised
Unmapped,
// These are mapped from the equivalent SES bounceSubType values
Undetermined,
General,
NoEmail,
Suppressed,
MailboxFull,
MessageTooLarge,
ContentRejected,
AttachmentRejected,
// These are mapped from the equivalent SES complaintFeedbackType values
Abuse,
AuthFailure,
Fraud,
NotSpam,
Other,
Virus,
}
impl<'d> Deserialize<'d> for BounceSubtype {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'d>,
{
let value: u8 = Deserialize::deserialize(deserializer)?;
match value {
0 => Ok(BounceSubtype::Unmapped),
1 => Ok(BounceSubtype::Undetermined),
2 => Ok(BounceSubtype::General),
3 => Ok(BounceSubtype::NoEmail),
4 => Ok(BounceSubtype::Suppressed),
5 => Ok(BounceSubtype::MailboxFull),
6 => Ok(BounceSubtype::MessageTooLarge),
7 => Ok(BounceSubtype::ContentRejected),
8 => Ok(BounceSubtype::AttachmentRejected),
9 => Ok(BounceSubtype::Abuse),
10 => Ok(BounceSubtype::AuthFailure),
11 => Ok(BounceSubtype::Fraud),
12 => Ok(BounceSubtype::NotSpam),
13 => Ok(BounceSubtype::Other),
14 => Ok(BounceSubtype::Virus),
_ => Err(D::Error::invalid_value(
Unexpected::Unsigned(u64::from(value)),
&"bounce subtype",
)),
}
}
}
impl Serialize for BounceSubtype {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let value = match self {
BounceSubtype::Unmapped => 0,
BounceSubtype::Undetermined => 1,
BounceSubtype::General => 2,
BounceSubtype::NoEmail => 3,
BounceSubtype::Suppressed => 4,
BounceSubtype::MailboxFull => 5,
BounceSubtype::MessageTooLarge => 6,
BounceSubtype::ContentRejected => 7,
BounceSubtype::AttachmentRejected => 8,
BounceSubtype::Abuse => 9,
BounceSubtype::AuthFailure => 10,
BounceSubtype::Fraud => 11,
BounceSubtype::NotSpam => 12,
BounceSubtype::Other => 13,
BounceSubtype::Virus => 14,
};
serializer.serialize_u8(value)
}
}
#[derive(Clone, Debug, Deserialize, Eq, Serialize, PartialEq)]
pub struct BounceRecord {
#[serde(rename = "email")]
pub address: EmailAddress,
#[serde(rename = "bounceType")]
pub bounce_type: BounceType,
#[serde(rename = "bounceSubType")]
pub bounce_subtype: BounceSubtype,
#[serde(rename = "createdAt")]
pub created_at: u64,
}
impl From<UrlError> for AppError {
fn from(error: UrlError) -> AppError {
AppErrorKind::AuthDbError(format!("{}", error)).into()
}
}
impl From<RequestError> for AppError {
fn from(error: RequestError) -> AppError {
AppErrorKind::AuthDbError(format!("{}", error)).into()
}
}
#[derive(Debug)]
struct DbUrls {
get_bounces: Url,
create_bounce: Url,
}
impl DbUrls {
pub fn new(settings: &Settings) -> DbUrls {
let base_uri: Url = settings.authdb.baseuri.0.parse().expect("invalid base URI");
DbUrls {
get_bounces: base_uri.join("emailBounces/").expect("invalid base URI"),
create_bounce: base_uri.join("emailBounces").expect("invalid base URI"),
}
}
pub fn get_bounces(&self, address: &EmailAddress) -> Result<Url, UrlError> {
self.get_bounces.join(&hex::encode(address.as_ref()))
}
pub fn create_bounce(&self) -> Url {
self.create_bounce.clone()
}
}
pub trait Db: Debug + Sync {
fn get_bounces(&self, address: &EmailAddress) -> AppResult<Vec<BounceRecord>>;
fn get_bounces(&self, address: &EmailAddress) -> AppResult<Vec<DeliveryProblem>>;
fn create_bounce(
&self,
_address: &EmailAddress,
_bounce_type: BounceType,
_bounce_subtype: BounceSubtype,
_problem_type: ProblemType,
_problem_subtype: ProblemSubtype,
) -> AppResult<()> {
Err(AppErrorKind::NotImplemented.into())
}
@@ -231,30 +58,30 @@ impl DbClient {
}
impl Db for DbClient {
fn get_bounces(&self, address: &EmailAddress) -> AppResult<Vec<BounceRecord>> {
fn get_bounces(&self, address: &EmailAddress) -> AppResult<Vec<DeliveryProblem>> {
let mut response = self
.request_client
.get(self.urls.get_bounces(address)?)
.send()?;
match response.status() {
StatusCode::Ok => response.json::<Vec<BounceRecord>>().map_err(From::from),
StatusCode::Ok => response.json::<Vec<DeliveryProblem>>().map_err(From::from),
status => Err(AppErrorKind::AuthDbError(format!("{}", status)).into()),
}
}
fn create_bounce(
&self,
address: &EmailAddress,
bounce_type: BounceType,
bounce_subtype: BounceSubtype,
problem_type: ProblemType,
problem_subtype: ProblemSubtype,
) -> AppResult<()> {
let response = self
.request_client
.post(self.urls.create_bounce())
.json(&BounceRecord {
.json(&DeliveryProblem {
address: address.clone(),
bounce_type,
bounce_subtype,
problem_type,
problem_subtype,
created_at: 0,
}).send()?;
match response.status() {
@@ -263,3 +90,39 @@ impl Db for DbClient {
}
}
}
#[derive(Debug)]
struct DbUrls {
get_bounces: Url,
create_bounce: Url,
}
impl DbUrls {
pub fn new(settings: &Settings) -> DbUrls {
let base_uri: Url = settings.authdb.baseuri.0.parse().expect("invalid base URI");
DbUrls {
get_bounces: base_uri.join("emailBounces/").expect("invalid base URI"),
create_bounce: base_uri.join("emailBounces").expect("invalid base URI"),
}
}
pub fn get_bounces(&self, address: &EmailAddress) -> Result<Url, UrlError> {
self.get_bounces.join(&hex::encode(address.as_ref()))
}
pub fn create_bounce(&self) -> Url {
self.create_bounce.clone()
}
}
impl From<UrlError> for AppError {
fn from(error: UrlError) -> AppError {
AppErrorKind::AuthDbError(format!("{}", error)).into()
}
}
impl From<RequestError> for AppError {
fn from(error: RequestError) -> AppError {
AppErrorKind::AuthDbError(format!("{}", error)).into()
}
}
Oops, something went wrong.

0 comments on commit 9f133af

Please sign in to comment.