`a2::response::Response` is confusing.

[ShaneQi]

a2::client::FutureResponse is a future whose Item type is a2::response::Response and Error type is a2::error::Error. Which gives me the impression that a2::response::Response will be a struct that carries the successful response of APNS. BUT a2::response::Response has the error property and it's code property could be failure codes (e.g. 410), and it's used in an error case a2::error:Error::ResponseError(a2::response::Response).

🤔️ So a2::response::Response is used in both successful cases and failure cases?

When I tried to handle errors, I were confused: if the APNS's reponse is 410, I should receive an a2::response::Response object whose code is 410.
But am I receiving it in Ok(a2::response::Response)?
Or am I receiving it in Err(a2::error:Error::ResponseError(a2::response::Response))?
(I assumed I'm gonna receive it in the latter, please remind me if I were wrong)

What makes more sense to me is that we split a2::response::Response into two structs (just a quick thought, can use better naming):

1. a2::response::SuccessResponse: a success response struct that only carries apns_id, and the success code (since 200 is the only one AFAIK, we might not need this property).
2. a2::response::FailureResponse: a failure response struct that carries the error code, apns_id, and error_body.

When APNS returns 200, future resolves to Ok(a2::response::SuccessResponse).
When APNS returns failure status codes, future resolves to Err(a2::error:Error::ResponseError(a2::response:: FailureResponse)).

[pimeys]

Hi,

It's been a while since I've worked with apple notifs and our company is no more. So I need to think a bit how my reasoning was here...

Our notification consumer is now also open source and here you see me mapping the errors: https://github.com/xray-tech/xorc-notifications/blob/master/src/apns2/consumer.rs#L127

• Ok is a successful notification and we don't do anything.
• Err(Error::ResponseError(e)) is an error from apple while processing the notification.
• Err(e) is an error that happened while connecting or transferring data. Something that is fatal.

Might not be so kosher anymore in 2019, but now if we're breaking the API, could you write a pull request as a proposition how you'd want to break it and we could discuss it more, allowing people already using the crate to also participate?

Maybe would be even nicer to integrate crates such as failure to the mix.

[ShaneQi]

Thanks for the response and the open mind.

I will open a PR if I can convert my thought into a reasonable change.

PS:

Thanks for the example code of your consumer, it helps a lot.