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.

Sign up for GitHub

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

Catch HTTP 411 link error #348

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Catch HTTP 411 link error #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/push_service/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ pub enum ServiceError {
#[error("Unknown CDN version {0}")]
UnknownCdnVersion(u32),

#[error("Device limit reached: {current} out of {max} devices.")]
DeviceLimitReached { current: u32, max: u32 },

#[error("HTTP reqwest error: {0}")]
Http(#[from] reqwest::Error),
}
21 changes: 21 additions & 0 deletions src/push_service/response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@ where
})?;
Err(ServiceError::ProofRequiredError(proof_required))
},
StatusCode::LENGTH_REQUIRED => {
#[derive(Debug, serde::Deserialize)]
struct LinkedDeviceNumberError {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem of doing this is that if any other endpoint gets 411 we'll deserialize the response to this type, this has happened with CONFLICT before. I wonder what we could do that works better? maybe infer the possible custom error type with a template parameter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what we could do that works better?

I'd just add a retry-chain, no? Or should we add some context information to the service_error_for_status function?

current: u32,
max: u32,
}
let error: LinkedDeviceNumberError =
response.json().await.map_err(|error| {
tracing::warn!(
%error,
"failed to decode linked device HTTP 411 status"
);
ServiceError::UnhandledResponseCode {
http_code: StatusCode::LENGTH_REQUIRED.as_u16(),
}
})?;
Err(ServiceError::DeviceLimitReached {
current: error.current,
max: error.max,
})
},
// XXX: fill in rest from PushServiceSocket
code => {
let response_text = response.text().await?;
Expand Down
Loading