log message when BackoffGuard is dropped without recording result

This commit is contained in:
Olivia Lee 2024-09-05 00:05:16 -07:00
parent 56f025cb47
commit 5bc3fce257
No known key found for this signature in database
GPG key ID: 54D568A15B9CD1F9

View file

@ -6,7 +6,7 @@ use std::{
use rand::{thread_rng, Rng}; use rand::{thread_rng, Rng};
use ruma::{OwnedServerName, ServerName}; use ruma::{OwnedServerName, ServerName};
use tracing::{debug, info, instrument}; use tracing::{debug, error, info, instrument};
use crate::{observability::METRICS, services, Error, Result}; use crate::{observability::METRICS, services, Error, Result};
@ -52,6 +52,7 @@ pub(crate) struct Service {
/// to it. /// to it.
#[must_use] #[must_use]
pub(crate) struct BackoffGuard { pub(crate) struct BackoffGuard {
result_recorded: bool,
backoff: Arc<RwLock<BackoffState>>, backoff: Arc<RwLock<BackoffState>>,
/// Store the last failure timestamp observed when this request started. If /// Store the last failure timestamp observed when this request started. If
/// there was another failure recorded since the request started, do not /// there was another failure recorded since the request started, do not
@ -127,6 +128,7 @@ impl Service {
}; };
Ok(BackoffGuard { Ok(BackoffGuard {
result_recorded: false,
backoff: state, backoff: state,
last_failure, last_failure,
}) })
@ -239,7 +241,9 @@ impl BackoffState {
impl BackoffGuard { impl BackoffGuard {
/// Record a successful request. /// Record a successful request.
#[instrument(skip(self))] #[instrument(skip(self))]
pub(crate) fn success(self) { pub(crate) fn success(mut self) {
self.result_recorded = true;
let mut state = self.backoff.write().unwrap(); let mut state = self.backoff.write().unwrap();
let was_online = state.is_online(); let was_online = state.is_online();
@ -266,7 +270,9 @@ impl BackoffGuard {
/// Examples of failures in this category are a timeout, a 500 status, or /// Examples of failures in this category are a timeout, a 500 status, or
/// a 404 from an endpoint that is not specced to return 404. /// a 404 from an endpoint that is not specced to return 404.
#[instrument(skip(self))] #[instrument(skip(self))]
pub(crate) fn hard_failure(self) { pub(crate) fn hard_failure(mut self) {
self.result_recorded = true;
let config = &services().globals.config.federation.backoff; let config = &services().globals.config.federation.backoff;
let mut state = self.backoff.write().unwrap(); let mut state = self.backoff.write().unwrap();
@ -299,9 +305,18 @@ impl BackoffGuard {
/// An example of a failure in this category is 404 from querying a user /// An example of a failure in this category is 404 from querying a user
/// profile. This might occur if the server no longer exists, but will also /// profile. This might occur if the server no longer exists, but will also
/// occur if the userid doesn't exist. /// occur if the userid doesn't exist.
// Taking `self` here is intentional, to allow callers to destroy the guard
// without triggering the `must_use` warning.
#[allow(clippy::unused_self)]
#[instrument(skip(self))] #[instrument(skip(self))]
pub(crate) fn soft_failure(self) {} pub(crate) fn soft_failure(mut self) {
self.result_recorded = true;
}
}
impl Drop for BackoffGuard {
fn drop(&mut self) {
if !self.result_recorded {
error!(
"BackoffGuard dropped without recording result. This is a bug."
);
}
}
} }